linux-ima-devel

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

[Peter M. <pm...@go...>]

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 | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 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..896c27b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -309,6 +309,45 @@ 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);
+
+	mutex_lock(&inode->i_mutex);
+	iint = integrity_iint_find(inode);
+
+	if (iint && (iint->flags & IMA_COLLECTED)) {
+		const char *algo_name = hash_algo_name[iint->ima_hash->algo];
+
+		snprintf(buf, size, "%s:%*phN",
+			algo_name, iint->ima_hash->length, iint->ima_hash->digest);
+
+		mutex_unlock(&inode->i_mutex);
+		return buf;
+	}
+
+	mutex_unlock(&inode->i_mutex);
+	return NULL;
+}
+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
  *
-- 
2.1.0.rc2.206.gedb03e5

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

[Peter M. <pm...@go...>]

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 | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 43 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..117ef18 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -309,6 +309,43 @@ 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;
+
+	buf = NULL;
+
+	if (!file)
+		goto out;
+
+	inode = file_inode(file);
+
+	mutex_lock(&inode->i_mutex);
+	iint = integrity_iint_find(inode);
+
+	if (iint && (iint->flags & IMA_COLLECTED))
+		snprintf(buf, size, "%s:%*phN",
+			hash_algo_name[iint->ima_hash->algo],
+			iint->ima_hash->length, iint->ima_hash->digest);
+
+	mutex_unlock(&inode->i_mutex);
+out:
+	return buf;
+}
+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
  *
-- 
2.1.0.rc2.206.gedb03e5

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

[Peter M. <pm...@go...>]

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);
+
+	mutex_lock(&inode->i_mutex);
+	iint = integrity_iint_find(inode);
+	if (iint && (iint->flags & IMA_COLLECTED))
+		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;
+}
+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
  *
-- 
2.1.0.rc2.206.gedb03e5

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

[Dmitry K. <d.k...@sa...>]

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.

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()'.

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
>   *

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

[Mimi Z. <zo...@li...>]

On Fri, 2014-11-07 at 15:50 +0200, Dmitry Kasatkin wrote: 
> Hi Peter,
> 
> We had a chat with Mimi and thought/discussed about couple of issues...

Thanks, Dmitry, for summarizing our discussion.

> 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.
> 
> 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.

In further discussion we realized that there is a major difference
between the two methods.  Searching the measurement list will return the
hash value(s) at the time the file was used, whereas opening the file to
query IMA could cause IMA to re-calculate the hash, if the file has
changed.

> > ---
> >  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...

With the above recommended changes, the 'iint' check is not needed
either.

Mimi

> > +		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()'.
> 
> 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
> >   *
> 

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

[Peter M. <pm...@go...>]

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.

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?


> 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
>>   *

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

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

[Mimi Z. <zo...@li...>]

On Mon, 2014-11-10 at 08:53 -0800, Peter Moody 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.
> 
> 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?

That's fine.  A motivation for this new feature needs to be documented
in the patch description. 

> > 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.

thanks!

Mimi

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

[Peter M. <pm...@go...>]

This allows other parts of the kernel (perhaps a stacked LSM focused on system
monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
calls to correlate network activity with the sending/receiving process) to
retrieve the hash of a given file from IMA if it's present in the iint cache.

It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
file was first measured a long time ago.

[1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
---
 include/linux/ima.h               |  6 ++++++
 security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 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..6a1cc80 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
+ * 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 (!ima_policy_flag || !file)
+		return ERR_PTR(-EINVAL);
+
+	inode = file_inode(file);
+
+	iint = integrity_iint_find(inode);
+	if (!iint)
+		return NULL;
+
+	mutex_lock(&inode->i_mutex);
+	if (iint->ima_hash->length > 0)
+		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;
+}
+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
  *
-- 
2.1.0.rc2.206.gedb03e5

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

[Dmitry K. <d.k...@sa...>]

On 13/11/14 21:09, Peter Moody wrote:
> This allows other parts of the kernel (perhaps a stacked LSM focused on system
> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
> calls to correlate network activity with the sending/receiving process) to
> retrieve the hash of a given file from IMA if it's present in the iint cache.
>
> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
> file was first measured a long time ago.
>
> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
> ---
>  include/linux/ima.h               |  6 ++++++
>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 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..6a1cc80 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
> + * 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 (!ima_policy_flag || !file)
> +		return ERR_PTR(-EINVAL);
> +

Hi,

Patch looks good.

May be few thoughts about error codes.
Do you want to return -EINVAL if IMA is disabled?
May be NULL as !iint? because of the same meaning.. inode is not handled
by IMA.

Also just want highlight that normally if you use ERR_PTR then client
checks error as IS_ERR(ptr).
NULL is not an error, just as 0.
So client need to check IS_ERR if it was error and NULL which is not an
error and means that inode is not handled by policy..

Is that what you want?
Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.

> +	inode = file_inode(file);
> +
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return NULL;
                      OK here.
> +
> +	mutex_lock(&inode->i_mutex);
> +	if (iint->ima_hash->length > 0)
> +		snprintf(buf, size, "%s:%*phN",
> +			hash_algo_name[iint->ima_hash->algo],
> +			iint->ima_hash->length, iint->ima_hash->digest);
> +	else
> +		buf = NULL;
                                   buf =  ERR_PTR(-EINVAL);
> +	mutex_unlock(&inode->i_mutex);
> +
> +	return buf;
> +}

Thanks

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
>   *

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

[Peter M. <pm...@go...>]

On Fri, Nov 14 2014 at 00:24, Dmitry Kasatkin wrote:
> On 13/11/14 21:09, Peter Moody wrote:
>> This allows other parts of the kernel (perhaps a stacked LSM focused on system
>> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
>> calls to correlate network activity with the sending/receiving process) to
>> retrieve the hash of a given file from IMA if it's present in the iint cache.
>>
>> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
>> file was first measured a long time ago.
>>
>> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
>> ---
>>  include/linux/ima.h               |  6 ++++++
>>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 45 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..6a1cc80 100644
>> --- a/security/integrity/ima/ima_main.c
>> +++ b/security/integrity/ima/ima_main.c
>> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
>> + * 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 (!ima_policy_flag || !file)
>> +		return ERR_PTR(-EINVAL);
>> +
>
> Hi,
>
> Patch looks good.
>
> May be few thoughts about error codes.
> Do you want to return -EINVAL if IMA is disabled?
> May be NULL as !iint? because of the same meaning.. inode is not handled
> by IMA.
>
> Also just want highlight that normally if you use ERR_PTR then client
> checks error as IS_ERR(ptr).
> NULL is not an error, just as 0.
> So client need to check IS_ERR if it was error and NULL which is not an
> error and means that inode is not handled by policy..
>
> Is that what you want?
> Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.

Following up on this patch (very very late), did you want me to update
the docstring referencing IS_ERR_OR_NULL() specifically or was this ok
as it stands? I'm fine with this returning IS_ERR_OR_NULL() if you all
are.

 Cheers,
 peter

-- 

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

[Peter M. <pm...@go...>]

On Mon, Mar 16 2015 at 10:25, Peter Moody wrote:
> On Fri, Nov 14 2014 at 00:24, Dmitry Kasatkin wrote:
>> On 13/11/14 21:09, Peter Moody wrote:
>>> This allows other parts of the kernel (perhaps a stacked LSM focused on system
>>> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
>>> calls to correlate network activity with the sending/receiving process) to
>>> retrieve the hash of a given file from IMA if it's present in the iint cache.
>>>
>>> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
>>> file was first measured a long time ago.
>>>
>>> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
>>> ---
>>>  include/linux/ima.h               |  6 ++++++
>>>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>  2 files changed, 45 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..6a1cc80 100644
>>> --- a/security/integrity/ima/ima_main.c
>>> +++ b/security/integrity/ima/ima_main.c
>>> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
>>> + * 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 (!ima_policy_flag || !file)
>>> +		return ERR_PTR(-EINVAL);
>>> +
>>
>> Hi,
>>
>> Patch looks good.
>>
>> May be few thoughts about error codes.
>> Do you want to return -EINVAL if IMA is disabled?
>> May be NULL as !iint? because of the same meaning.. inode is not handled
>> by IMA.
>>
>> Also just want highlight that normally if you use ERR_PTR then client
>> checks error as IS_ERR(ptr).
>> NULL is not an error, just as 0.
>> So client need to check IS_ERR if it was error and NULL which is not an
>> error and means that inode is not handled by policy..
>>
>> Is that what you want?
>> Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.

(re-pining because I'm not sure if my last message made it through).

> Following up on this patch (very very late), did you want me to update
> the docstring referencing IS_ERR_OR_NULL() specifically or was this ok
> as it stands? I'm fine with this returning IS_ERR_OR_NULL() if you all
> are.
>
>  Cheers,
>  peter

-- 

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

[Mimi Z. <zo...@li...>]

On Mon, 2015-03-30 at 19:14 -0700, Peter Moody wrote: 
> On Mon, Mar 16 2015 at 10:25, Peter Moody wrote:
> > On Fri, Nov 14 2014 at 00:24, Dmitry Kasatkin wrote:
> >> On 13/11/14 21:09, Peter Moody wrote:
> >>> This allows other parts of the kernel (perhaps a stacked LSM focused on system
> >>> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
> >>> calls to correlate network activity with the sending/receiving process) to
> >>> retrieve the hash of a given file from IMA if it's present in the iint cache.
> >>>
> >>> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
> >>> file was first measured a long time ago.
> >>>
> >>> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
> >>> ---
> >>>  include/linux/ima.h               |  6 ++++++
> >>>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
> >>>  2 files changed, 45 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..6a1cc80 100644
> >>> --- a/security/integrity/ima/ima_main.c
> >>> +++ b/security/integrity/ima/ima_main.c
> >>> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
> >>> + * 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 (!ima_policy_flag || !file)
> >>> +		return ERR_PTR(-EINVAL);
> >>> +
> >>
> >> Hi,
> >>
> >> Patch looks good.
> >>
> >> May be few thoughts about error codes.
> >> Do you want to return -EINVAL if IMA is disabled?
> >> May be NULL as !iint? because of the same meaning.. inode is not handled
> >> by IMA.
> >>
> >> Also just want highlight that normally if you use ERR_PTR then client
> >> checks error as IS_ERR(ptr).
> >> NULL is not an error, just as 0.
> >> So client need to check IS_ERR if it was error and NULL which is not an
> >> error and means that inode is not handled by policy..
> >>
> >> Is that what you want?
> >> Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.
> 
> (re-pining because I'm not sure if my last message made it through).

Sorry for the delay in responding...  Let me refresh my memory a bit
before commenting.

Mimi

> > Following up on this patch (very very late), did you want me to update
> > the docstring referencing IS_ERR_OR_NULL() specifically or was this ok
> > as it stands? I'm fine with this returning IS_ERR_OR_NULL() if you all
> > are.

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

[Mimi Z. <zo...@li...>]

On Mon, 2015-03-30 at 22:24 -0400, Mimi Zohar wrote:
> On Mon, 2015-03-30 at 19:14 -0700, Peter Moody wrote: 
> > On Mon, Mar 16 2015 at 10:25, Peter Moody wrote:
> > > On Fri, Nov 14 2014 at 00:24, Dmitry Kasatkin wrote:
> > >> On 13/11/14 21:09, Peter Moody wrote:
> > >>> This allows other parts of the kernel (perhaps a stacked LSM focused on system
> > >>> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
> > >>> calls to correlate network activity with the sending/receiving process) to
> > >>> retrieve the hash of a given file from IMA if it's present in the iint cache.
> > >>>
> > >>> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
> > >>> file was first measured a long time ago.
> > >>>
> > >>> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
> > >>> ---
> > >>>  include/linux/ima.h               |  6 ++++++
> > >>>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >>>  2 files changed, 45 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..6a1cc80 100644
> > >>> --- a/security/integrity/ima/ima_main.c
> > >>> +++ b/security/integrity/ima/ima_main.c
> > >>> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
> > >>> + * 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 (!ima_policy_flag || !file)
> > >>> +		return ERR_PTR(-EINVAL);
> > >>> +
> > >>
> > >> Hi,
> > >>
> > >> Patch looks good.
> > >>
> > >> May be few thoughts about error codes.
> > >> Do you want to return -EINVAL if IMA is disabled?
> > >> May be NULL as !iint? because of the same meaning.. inode is not handled
> > >> by IMA.
> > >>
> > >> Also just want highlight that normally if you use ERR_PTR then client
> > >> checks error as IS_ERR(ptr).
> > >> NULL is not an error, just as 0.
> > >> So client need to check IS_ERR if it was error and NULL which is not an
> > >> error and means that inode is not handled by policy..
> > >>
> > >> Is that what you want?
> > >> Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.
> > 
> > (re-pining because I'm not sure if my last message made it through).
> 
> Sorry for the delay in responding...  Let me refresh my memory a bit
> before commenting.
> 
> Mimi
> 
> > > Following up on this patch (very very late), did you want me to update
> > > the docstring referencing IS_ERR_OR_NULL() specifically or was this ok
> > > as it stands? I'm fine with this returning IS_ERR_OR_NULL() if you all
> > > are.

The IMA (and security) hooks normally return 0 on success and an error
code otherwise.  Is there any benefit in returning 'buf'?

Mimi

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

[Peter M. <pm...@go...>]

On Sun, Apr 26 2015 at 13:13, Mimi Zohar wrote:
> On Mon, 2015-03-30 at 22:24 -0400, Mimi Zohar wrote:
>> On Mon, 2015-03-30 at 19:14 -0700, Peter Moody wrote: 
>> > On Mon, Mar 16 2015 at 10:25, Peter Moody wrote:
>> > > On Fri, Nov 14 2014 at 00:24, Dmitry Kasatkin wrote:
>> > >> On 13/11/14 21:09, Peter Moody wrote:
>> > >>> This allows other parts of the kernel (perhaps a stacked LSM focused on system
>> > >>> monitoring, eg. the proposed hone LSM [1] designed to monitor fork/exec/exit
>> > >>> calls to correlate network activity with the sending/receiving process) to
>> > >>> retrieve the hash of a given file from IMA if it's present in the iint cache.
>> > >>>
>> > >>> It's true that the existence of the hash means that it's also 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 given host has been up for a long time and the
>> > >>> file was first measured a long time ago.
>> > >>>
>> > >>> [1] http://marc.info/?l=linux-security-module&m=140685613922280&w=2
>> > >>> ---
>> > >>>  include/linux/ima.h               |  6 ++++++
>> > >>>  security/integrity/ima/ima_main.c | 39 +++++++++++++++++++++++++++++++++++++++
>> > >>>  2 files changed, 45 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..6a1cc80 100644
>> > >>> --- a/security/integrity/ima/ima_main.c
>> > >>> +++ b/security/integrity/ima/ima_main.c
>> > >>> @@ -309,6 +309,45 @@ 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. If IMA is disabled, return ERR_PTR(-EVINVAL).
>> > >>> + * 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 (!ima_policy_flag || !file)
>> > >>> +		return ERR_PTR(-EINVAL);
>> > >>> +
>> > >>
>> > >> Hi,
>> > >>
>> > >> Patch looks good.
>> > >>
>> > >> May be few thoughts about error codes.
>> > >> Do you want to return -EINVAL if IMA is disabled?
>> > >> May be NULL as !iint? because of the same meaning.. inode is not handled
>> > >> by IMA.
>> > >>
>> > >> Also just want highlight that normally if you use ERR_PTR then client
>> > >> checks error as IS_ERR(ptr).
>> > >> NULL is not an error, just as 0.
>> > >> So client need to check IS_ERR if it was error and NULL which is not an
>> > >> error and means that inode is not handled by policy..
>> > >>
>> > >> Is that what you want?
>> > >> Or if NULL will be used as error, then IS_ERR_OR_NULL(ptr) can be used.
>> > 
>> > (re-pining because I'm not sure if my last message made it through).
>> 
>> Sorry for the delay in responding...  Let me refresh my memory a bit
>> before commenting.
>> 
>> Mimi
>> 
>> > > Following up on this patch (very very late), did you want me to update
>> > > the docstring referencing IS_ERR_OR_NULL() specifically or was this ok
>> > > as it stands? I'm fine with this returning IS_ERR_OR_NULL() if you all
>> > > are.
>
> The IMA (and security) hooks normally return 0 on success and an error
> code otherwise.  Is there any benefit in returning 'buf'?

I was originally thinking that it would nice to be able to call
something like

  pr_info("hash %s\n", ima_file_hash(file, buf, size));

but I have to check the return value before I use it anyway, so no,
there's no real benefit. I'll update with the 0/1 return value.

> Mimi
>

-- 
peter moody

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

[Mimi Z. <zo...@li...>]

On Tue, 2014-11-04 at 15:17 -0800, 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 | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 45 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..896c27b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -309,6 +309,45 @@ 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);
> +
> +	mutex_lock(&inode->i_mutex);
> +	iint = integrity_iint_find(inode);
> +
> +	if (iint && (iint->flags & IMA_COLLECTED)) {
> +		const char *algo_name = hash_algo_name[iint->ima_hash->algo];
> +
> +		snprintf(buf, size, "%s:%*phN",
> +			algo_name, iint->ima_hash->length, iint->ima_hash->digest);
> +
> +		mutex_unlock(&inode->i_mutex);
> +		return buf;

There's no additional code between here and the function exit.  How
about a single exit point?

Mimi

> +	}
> +
> +	mutex_unlock(&inode->i_mutex);
> +	return NULL;
> +}
> +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
>   *