[v2] ima: add the ability to query the hash of a given file.

Commit Message

Florent Revest Jan. 6, 2020, 4:25 p.m. UTC

From: Florent Revest <[e-mail]>

This allows other parts of the kernel (perhaps a stacked LSM allowing
system monitoring, eg. the proposed KRSI LSM [1]) 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 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.

This is based on Peter Moody's patch:
 https://sourceforge.net/p/linux-ima/mailman/message/33036180/

[1] https://lkml.org/lkml/2019/9/10/393

Signed-off-by: Florent Revest <[e-mail]>
---
 include/linux/ima.h               |  6 ++++
 security/integrity/ima/ima_main.c | 46 +++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+)

Comments

Mimi Zohar Jan. 8, 2020, 4:05 p.m. UTC | #1

On Mon, 2020-01-06 at 17:25 +0100, Florent Revest wrote:
> From: Florent Revest <[e-mail]>
> 
> This allows other parts of the kernel (perhaps a stacked LSM allowing
> system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> 
> This is based on Peter Moody's patch:
>  https://sourceforge.net/p/linux-ima/mailman/message/33036180/

FYI, but unlike the audit log/IMA measurement list, the iint cache
entries can be removed.  Refer to security_inode_free().  Perhaps
mention of this difference should be included, here, in the patch
description.

> 
> [1] https://lkml.org/lkml/2019/9/10/393
> 
> Signed-off-by: Florent Revest <[e-mail]>

Assuming, with the above difference, you're still interested in having
this feature upstreamed and addressing the comments above and below:

Reviewed-by: Mimi Zohar <[e-mail]>

> ---
>  include/linux/ima.h               |  6 ++++
>  security/integrity/ima/ima_main.c | 46 +++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..d621c65ba9a5 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
>  extern void ima_kexec_cmdline(const void *buf, int size);
>  
>  #ifdef CONFIG_IMA_KEXEC
> @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static inline void ima_kexec_cmdline(const void *buf, int size) {}
>  #endif /* CONFIG_IMA */
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..3799b6c6c3b8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int mask)
>  }
>  EXPORT_SYMBOL_GPL(ima_file_check);
>  
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, return the hash algorithm (as defined in the enum hash_algo).
> + * If buf is not NULL, this function also outputs the hash into buf.

As of Linux 5.4.y, IMA support for appended file signatures was added.
 Should we indicate that the file hash returned is based on the entire
file, including the appended signature?

Mimi


> + * If the hash is larger than buf_size, then only buf_size bytes will be copied.
> + * It generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE
> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> +	struct inode *inode;
> +	struct integrity_iint_cache *iint;
> +	int hash_algo;
> +
> +	if (!file)
> +		return -EINVAL;
> +
> +	if (!ima_policy_flag)
> +		return -EOPNOTSUPP;
> +
> +	inode = file_inode(file);
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&iint->mutex);
> +	if (buf) {
> +		size_t copied_size;
> +
> +		copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> +		memcpy(buf, iint->ima_hash->digest, copied_size);
> +	}
> +	hash_algo = iint->ima_hash->algo;
> +	mutex_unlock(&iint->mutex);
> +
> +	return hash_algo;
> +}
> +EXPORT_SYMBOL_GPL(ima_file_hash);
> +
>  /**
>   * ima_post_create_tmpfile - mark newly created tmpfile as new
>   * @file : newly created tmpfile

Florent Revest Jan. 13, 2020, 9:42 a.m. UTC | #2

On Wed, 2020-01-08 at 11:05 -0500, Mimi Zohar wrote:
> On Mon, 2020-01-06 at 17:25 +0100, Florent Revest wrote:
> > From: Florent Revest <[e-mail]>
> > 
> > This allows other parts of the kernel (perhaps a stacked LSM
> > allowing
> > system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> > 
> > This is based on Peter Moody's patch:
> >  https://sourceforge.net/p/linux-ima/mailman/message/33036180/
> 
> FYI, but unlike the audit log/IMA measurement list, the iint cache
> entries can be removed.  Refer to security_inode_free().  Perhaps
> mention of this difference should be included, here, in the patch
> description.

Sure, I added a comment about this in a v3.

> > [1] https://lkml.org/lkml/2019/9/10/393
> > 
> > Signed-off-by: Florent Revest <[e-mail]>
> 
> Assuming, with the above difference, you're still interested in
> having this feature upstreamed and addressing the comments above and
> below:
> 
> Reviewed-by: Mimi Zohar <[e-mail]>

Thank you. Yes we are still interested in this feature!

> > ---
> >  include/linux/ima.h               |  6 ++++
> >  security/integrity/ima/ima_main.c | 46
> > +++++++++++++++++++++++++++++++
> >  2 files changed, 52 insertions(+)
> > 
> > diff --git a/include/linux/ima.h b/include/linux/ima.h
> > index 6d904754d858..d621c65ba9a5 100644
> > --- a/include/linux/ima.h
> > +++ b/include/linux/ima.h
> > @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum
> > kernel_read_file_id id);
> >  extern int ima_post_read_file(struct file *file, void *buf, loff_t
> > size,
> >  			      enum kernel_read_file_id id);
> >  extern void ima_post_path_mknod(struct dentry *dentry);
> > +extern int ima_file_hash(struct file *file, char *buf, size_t
> > buf_size);
> >  extern void ima_kexec_cmdline(const void *buf, int size);
> >  
> >  #ifdef CONFIG_IMA_KEXEC
> > @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct
> > dentry *dentry)
> >  	return;
> >  }
> >  
> > +static inline int ima_file_hash(struct file *file, char *buf,
> > size_t buf_size)
> > +{
> > +	return -EOPNOTSUPP;
> > +}
> > +
> >  static inline void ima_kexec_cmdline(const void *buf, int size) {}
> >  #endif /* CONFIG_IMA */
> >  
> > diff --git a/security/integrity/ima/ima_main.c
> > b/security/integrity/ima/ima_main.c
> > index d7e987baf127..3799b6c6c3b8 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int
> > mask)
> >  }
> >  EXPORT_SYMBOL_GPL(ima_file_check);
> >  
> > +/**
> > + * ima_file_hash - return the stored measurement if a file has
> > been hashed.
> > + * @file: pointer to the file
> > + * @buf: buffer in which to store the hash
> > + * @buf_size: length of the buffer
> > + *
> > + * On success, return the hash algorithm (as defined in the enum
> > hash_algo).
> > + * If buf is not NULL, this function also outputs the hash into
> > buf.
> 
> As of Linux 5.4.y, IMA support for appended file signatures was
> added. Should we indicate that the file hash returned is based on the
> entire file, including the appended signature?
> 
> Mimi

Of course it never hurts to add a comment. :) I'll send a v3 with this
added.

> 
> > + * If the hash is larger than buf_size, then only buf_size bytes
> > will be copied.
> > + * It generally just makes sense to pass a buffer capable of
> > holding the largest
> > + * possible hash: IMA_MAX_DIGEST_SIZE
> > + *
> > + * If IMA is disabled or if no measurement is available, return
> > -EOPNOTSUPP.
> > + * If the parameters are incorrect, return -EINVAL.
> > + */
> > +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> > +{
> > +	struct inode *inode;
> > +	struct integrity_iint_cache *iint;
> > +	int hash_algo;
> > +
> > +	if (!file)
> > +		return -EINVAL;
> > +
> > +	if (!ima_policy_flag)
> > +		return -EOPNOTSUPP;
> > +
> > +	inode = file_inode(file);
> > +	iint = integrity_iint_find(inode);
> > +	if (!iint)
> > +		return -EOPNOTSUPP;
> > +
> > +	mutex_lock(&iint->mutex);
> > +	if (buf) {
> > +		size_t copied_size;
> > +
> > +		copied_size = min_t(size_t, iint->ima_hash->length,
> > buf_size);
> > +		memcpy(buf, iint->ima_hash->digest, copied_size);
> > +	}
> > +	hash_algo = iint->ima_hash->algo;
> > +	mutex_unlock(&iint->mutex);
> > +
> > +	return hash_algo;
> > +}
> > +EXPORT_SYMBOL_GPL(ima_file_hash);
> > +
> >  /**
> >   * ima_post_create_tmpfile - mark newly created tmpfile as new
> >   * @file : newly created tmpfile

KP Singh Jan. 13, 2020, 10:48 a.m. UTC | #3

On 06-Jan 17:25, Florent Revest wrote:
> From: Florent Revest <[e-mail]>
> 
> This allows other parts of the kernel (perhaps a stacked LSM allowing
> system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> 
> This is based on Peter Moody's patch:
>  https://sourceforge.net/p/linux-ima/mailman/message/33036180/
> 
> [1] https://lkml.org/lkml/2019/9/10/393
> 
> Signed-off-by: Florent Revest <[e-mail]>

Thanks for adding this Florent!

Reviewed-by: KP Singh <[e-mail]>

> ---
>  include/linux/ima.h               |  6 ++++
>  security/integrity/ima/ima_main.c | 46 +++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 6d904754d858..d621c65ba9a5 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -23,6 +23,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
>  extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
>  			      enum kernel_read_file_id id);
>  extern void ima_post_path_mknod(struct dentry *dentry);
> +extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
>  extern void ima_kexec_cmdline(const void *buf, int size);
>  
>  #ifdef CONFIG_IMA_KEXEC
> @@ -91,6 +92,11 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
>  	return;
>  }
>  
> +static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  static inline void ima_kexec_cmdline(const void *buf, int size) {}
>  #endif /* CONFIG_IMA */
>  
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index d7e987baf127..3799b6c6c3b8 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -445,6 +445,52 @@ int ima_file_check(struct file *file, int mask)
>  }
>  EXPORT_SYMBOL_GPL(ima_file_check);
>  
> +/**
> + * ima_file_hash - return the stored measurement if a file has been hashed.
> + * @file: pointer to the file
> + * @buf: buffer in which to store the hash
> + * @buf_size: length of the buffer
> + *
> + * On success, return the hash algorithm (as defined in the enum hash_algo).
> + * If buf is not NULL, this function also outputs the hash into buf.
> + * If the hash is larger than buf_size, then only buf_size bytes will be copied.
> + * It generally just makes sense to pass a buffer capable of holding the largest
> + * possible hash: IMA_MAX_DIGEST_SIZE
> + *
> + * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
> + * If the parameters are incorrect, return -EINVAL.
> + */
> +int ima_file_hash(struct file *file, char *buf, size_t buf_size)
> +{
> +	struct inode *inode;
> +	struct integrity_iint_cache *iint;
> +	int hash_algo;
> +
> +	if (!file)
> +		return -EINVAL;
> +
> +	if (!ima_policy_flag)
> +		return -EOPNOTSUPP;
> +
> +	inode = file_inode(file);
> +	iint = integrity_iint_find(inode);
> +	if (!iint)
> +		return -EOPNOTSUPP;
> +
> +	mutex_lock(&iint->mutex);
> +	if (buf) {
> +		size_t copied_size;
> +
> +		copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
> +		memcpy(buf, iint->ima_hash->digest, copied_size);
> +	}
> +	hash_algo = iint->ima_hash->algo;
> +	mutex_unlock(&iint->mutex);
> +
> +	return hash_algo;
> +}
> +EXPORT_SYMBOL_GPL(ima_file_hash);
> +
>  /**
>   * ima_post_create_tmpfile - mark newly created tmpfile as new
>   * @file : newly created tmpfile
> -- 
> 2.24.1.735.g03f4e72817-goog
>

Mimi Zohar Jan. 15, 2020, 6:36 p.m. UTC | #4

On Mon, 2020-01-13 at 11:48 +0100, KP Singh wrote:
> On 06-Jan 17:25, Florent Revest wrote:
> > From: Florent Revest <[e-mail]>
> > 
> > This allows other parts of the kernel (perhaps a stacked LSM allowing
> > system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> > 
> > This is based on Peter Moody's patch:
> >  https://sourceforge.net/p/linux-ima/mailman/message/33036180/
> > 
> > [1] https://lkml.org/lkml/2019/9/10/393
> > 
> > Signed-off-by: Florent Revest <[e-mail]>
> 
> Thanks for adding this Florent!
> 
> Reviewed-by: KP Singh <[e-mail]>

Thanks, this patch is now queued in next-integrity-testing.

Mimi

Florent Revest Jan. 15, 2020, 6:45 p.m. UTC | #5

On Wed, 2020-01-15 at 13:36 -0500, Mimi Zohar wrote:
> On Mon, 2020-01-13 at 11:48 +0100, KP Singh wrote:
> > On 06-Jan 17:25, Florent Revest wrote:
> > > From: Florent Revest <[e-mail]>
> > > 
> > > This allows other parts of the kernel (perhaps a stacked LSM
> > > allowing
> > > system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> > > 
> > > This is based on Peter Moody's patch:
> > >  https://sourceforge.net/p/linux-ima/mailman/message/33036180/
> > > 
> > > [1] https://lkml.org/lkml/2019/9/10/393
> > > 
> > > Signed-off-by: Florent Revest <[e-mail]>
> > 
> > Thanks for adding this Florent!
> > 
> > Reviewed-by: KP Singh <[e-mail]>
> 
> Thanks, this patch is now queued in next-integrity-testing.

Good to hear Mimi! Thank you.

I would just like to make sure that you queued the v3 of this patch
though...? (this thread is for the v2 :) ) The v3 includes a couple of
comments you asked for.

Mimi Zohar Jan. 15, 2020, 7:09 p.m. UTC | #6

On Wed, 2020-01-15 at 19:45 +0100, Florent Revest wrote:
> On Wed, 2020-01-15 at 13:36 -0500, Mimi Zohar wrote:
> > On Mon, 2020-01-13 at 11:48 +0100, KP Singh wrote:
> > > On 06-Jan 17:25, Florent Revest wrote:
> > > > From: Florent Revest <[e-mail]>
> > > > 
> > > > This allows other parts of the kernel (perhaps a stacked LSM
> > > > allowing
> > > > system monitoring, eg. the proposed KRSI LSM [1]) 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 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.
> > > > 
> > > > This is based on Peter Moody's patch:
> > > >  https://sourceforge.net/p/linux-ima/mailman/message/33036180/
> > > > 
> > > > [1] https://lkml.org/lkml/2019/9/10/393
> > > > 
> > > > Signed-off-by: Florent Revest <[e-mail]>
> > > 
> > > Thanks for adding this Florent!
> > > 
> > > Reviewed-by: KP Singh <[e-mail]>
> > 
> > Thanks, this patch is now queued in next-integrity-testing.
> 
> Good to hear Mimi! Thank you.
> 
> I would just like to make sure that you queued the v3 of this patch
> though...? (this thread is for the v2 :) ) The v3 includes a couple of
> comments you asked for.

Oops, yes v3 is queued.

Mimi

Patch

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 6d904754d858..d621c65ba9a5 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -23,6 +23,7 @@  extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
 extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
+extern int ima_file_hash(struct file *file, char *buf, size_t buf_size);
 extern void ima_kexec_cmdline(const void *buf, int size);
 
 #ifdef CONFIG_IMA_KEXEC
@@ -91,6 +92,11 @@  static inline void ima_post_path_mknod(struct dentry *dentry)
 	return;
 }
 
+static inline int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void ima_kexec_cmdline(const void *buf, int size) {}
 #endif /* CONFIG_IMA */
 
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index d7e987baf127..3799b6c6c3b8 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -445,6 +445,52 @@  int ima_file_check(struct file *file, int mask)
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
 
+/**
+ * ima_file_hash - return the stored measurement if a file has been hashed.
+ * @file: pointer to the file
+ * @buf: buffer in which to store the hash
+ * @buf_size: length of the buffer
+ *
+ * On success, return the hash algorithm (as defined in the enum hash_algo).
+ * If buf is not NULL, this function also outputs the hash into buf.
+ * If the hash is larger than buf_size, then only buf_size bytes will be copied.
+ * It generally just makes sense to pass a buffer capable of holding the largest
+ * possible hash: IMA_MAX_DIGEST_SIZE
+ *
+ * If IMA is disabled or if no measurement is available, return -EOPNOTSUPP.
+ * If the parameters are incorrect, return -EINVAL.
+ */
+int ima_file_hash(struct file *file, char *buf, size_t buf_size)
+{
+	struct inode *inode;
+	struct integrity_iint_cache *iint;
+	int hash_algo;
+
+	if (!file)
+		return -EINVAL;
+
+	if (!ima_policy_flag)
+		return -EOPNOTSUPP;
+
+	inode = file_inode(file);
+	iint = integrity_iint_find(inode);
+	if (!iint)
+		return -EOPNOTSUPP;
+
+	mutex_lock(&iint->mutex);
+	if (buf) {
+		size_t copied_size;
+
+		copied_size = min_t(size_t, iint->ima_hash->length, buf_size);
+		memcpy(buf, iint->ima_hash->digest, copied_size);
+	}
+	hash_algo = iint->ima_hash->algo;
+	mutex_unlock(&iint->mutex);
+
+	return hash_algo;
+}
+EXPORT_SYMBOL_GPL(ima_file_hash);
+
 /**
  * ima_post_create_tmpfile - mark newly created tmpfile as new
  * @file : newly created tmpfile