2022-02-12 18:24:25

by Mimi Zohar

[permalink] [raw]
Subject: [PATCH v5 3/8] fs-verity: define a function to return the integrity protected file digest

Define a function named fsverity_get_digest() to return the verity file
digest and the associated hash algorithm (enum hash_algo).

Acked-by: Eric Biggers <[email protected]>
Signed-off-by: Mimi Zohar <[email protected]>
---
fs/verity/Kconfig | 1 +
fs/verity/fsverity_private.h | 7 ------
fs/verity/measure.c | 41 ++++++++++++++++++++++++++++++++++++
include/linux/fsverity.h | 18 ++++++++++++++++
4 files changed, 60 insertions(+), 7 deletions(-)

diff --git a/fs/verity/Kconfig b/fs/verity/Kconfig
index 24d1b54de807..54598cd80145 100644
--- a/fs/verity/Kconfig
+++ b/fs/verity/Kconfig
@@ -3,6 +3,7 @@
config FS_VERITY
bool "FS Verity (read-only file-based authenticity protection)"
select CRYPTO
+ select CRYPTO_HASH_INFO
# SHA-256 is implied as it's intended to be the default hash algorithm.
# To avoid bloat, other wanted algorithms must be selected explicitly.
# Note that CRYPTO_SHA256 denotes the generic C implementation, but
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index a7920434bae5..c6fb62e0ef1a 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -14,7 +14,6 @@

#define pr_fmt(fmt) "fs-verity: " fmt

-#include <crypto/sha2.h>
#include <linux/fsverity.h>
#include <linux/mempool.h>

@@ -26,12 +25,6 @@ struct ahash_request;
*/
#define FS_VERITY_MAX_LEVELS 8

-/*
- * Largest digest size among all hash algorithms supported by fs-verity.
- * Currently assumed to be <= size of fsverity_descriptor::root_hash.
- */
-#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
-
/* A hash algorithm supported by fs-verity */
struct fsverity_hash_alg {
struct crypto_ahash *tfm; /* hash tfm, allocated on demand */
diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index f0d7b30c62db..f832aaa41326 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -57,3 +57,44 @@ int fsverity_ioctl_measure(struct file *filp, void __user *_uarg)
return 0;
}
EXPORT_SYMBOL_GPL(fsverity_ioctl_measure);
+
+/**
+ * fsverity_get_digest() - get a verity file's digest
+ * @inode: inode to get digest of
+ * @digest: (out) pointer to the digest
+ * @alg: (out) pointer to the hash algorithm enumeration
+ *
+ * Return the file hash algorithm and digest of an fsverity protected file.
+ *
+ * Return: 0 on success, -errno on failure
+ */
+int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg)
+{
+ const struct fsverity_info *vi;
+ const struct fsverity_hash_alg *hash_alg;
+ int i;
+
+ vi = fsverity_get_info(inode);
+ if (!vi)
+ return -ENODATA; /* not a verity file */
+
+ hash_alg = vi->tree_params.hash_alg;
+ memset(digest, 0, FS_VERITY_MAX_DIGEST_SIZE);
+
+ /* convert the verity hash algorithm name to a hash_algo_name enum */
+ i = match_string(hash_algo_name, HASH_ALGO__LAST, hash_alg->name);
+ if (i < 0)
+ return -EINVAL;
+ *alg = i;
+
+ if (WARN_ON_ONCE(hash_alg->digest_size != hash_digest_size[*alg]))
+ return -EINVAL;
+ memcpy(digest, vi->file_digest, hash_alg->digest_size);
+
+ pr_debug("file digest %s:%*phN\n", hash_algo_name[*alg],
+ hash_digest_size[*alg], digest);
+
+ return 0;
+}
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index b568b3c7d095..9a1b70cc7318 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -12,8 +12,16 @@
#define _LINUX_FSVERITY_H

#include <linux/fs.h>
+#include <crypto/hash_info.h>
+#include <crypto/sha2.h>
#include <uapi/linux/fsverity.h>

+/*
+ * Largest digest size among all hash algorithms supported by fs-verity.
+ * Currently assumed to be <= size of fsverity_descriptor::root_hash.
+ */
+#define FS_VERITY_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE
+
/* Verity operations for filesystems */
struct fsverity_operations {

@@ -131,6 +139,9 @@ int fsverity_ioctl_enable(struct file *filp, const void __user *arg);
/* measure.c */

int fsverity_ioctl_measure(struct file *filp, void __user *arg);
+int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg);

/* open.c */

@@ -170,6 +181,13 @@ static inline int fsverity_ioctl_measure(struct file *filp, void __user *arg)
return -EOPNOTSUPP;
}

+static inline int fsverity_get_digest(struct inode *inode,
+ u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
+ enum hash_algo *alg)
+{
+ return -EOPNOTSUPP;
+}
+
/* open.c */

static inline int fsverity_file_open(struct inode *inode, struct file *filp)
--
2.27.0


2022-02-24 01:26:15

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the integrity protected file digest

On Fri, Feb 11, 2022 at 04:43:05PM -0500, Mimi Zohar wrote:
> +/**
> + * fsverity_get_digest() - get a verity file's digest
> + * @inode: inode to get digest of
> + * @digest: (out) pointer to the digest
> + * @alg: (out) pointer to the hash algorithm enumeration
> + *
> + * Return the file hash algorithm and digest of an fsverity protected file.
> + *
> + * Return: 0 on success, -errno on failure
> + */
> +int fsverity_get_digest(struct inode *inode,
> + u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> + enum hash_algo *alg)
> +{
> + const struct fsverity_info *vi;
> + const struct fsverity_hash_alg *hash_alg;
> + int i;
> +
> + vi = fsverity_get_info(inode);
> + if (!vi)
> + return -ENODATA; /* not a verity file */

Sorry for the slow reviews; I'm taking a look again now. One question about
something I missed earlier: is the file guaranteed to have been opened before
this is called? fsverity_get_info() only returns a non-NULL value if the file
has been opened at least once since the inode has been loaded into memory. If
the inode has just been loaded into memory without being opened, for example due
to a call to stat(), then fsverity_get_info() will return NULL.

If the file is guaranteed to have been opened, then the code is fine, but the
comment for fsverity_get_digest() perhaps should be updated to mention this
assumption, given that it takes a struct inode rather than a struct file.

If the file is *not* guaranteed to have been opened, then it would be necessary
to make fsverity_get_digest() call ensure_verity_info() to set up the
fsverity_info.

- Eric

2022-02-24 02:07:54

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the integrity protected file digest

On Wed, Feb 23, 2022 at 08:21:01PM -0500, Mimi Zohar wrote:
> On Wed, 2022-02-23 at 15:59 -0800, Eric Biggers wrote:
> > On Fri, Feb 11, 2022 at 04:43:05PM -0500, Mimi Zohar wrote:
> > > +/**
> > > + * fsverity_get_digest() - get a verity file's digest
> > > + * @inode: inode to get digest of
> > > + * @digest: (out) pointer to the digest
> > > + * @alg: (out) pointer to the hash algorithm enumeration
> > > + *
> > > + * Return the file hash algorithm and digest of an fsverity protected file.
> > > + *
> > > + * Return: 0 on success, -errno on failure
> > > + */
> > > +int fsverity_get_digest(struct inode *inode,
> > > + u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> > > + enum hash_algo *alg)
> > > +{
> > > + const struct fsverity_info *vi;
> > > + const struct fsverity_hash_alg *hash_alg;
> > > + int i;
> > > +
> > > + vi = fsverity_get_info(inode);
> > > + if (!vi)
> > > + return -ENODATA; /* not a verity file */
> >
> > Sorry for the slow reviews; I'm taking a look again now. One question about
> > something I missed earlier: is the file guaranteed to have been opened before
> > this is called? fsverity_get_info() only returns a non-NULL value if the file
> > has been opened at least once since the inode has been loaded into memory. If
> > the inode has just been loaded into memory without being opened, for example due
> > to a call to stat(), then fsverity_get_info() will return NULL.
> >
> > If the file is guaranteed to have been opened, then the code is fine, but the
> > comment for fsverity_get_digest() perhaps should be updated to mention this
> > assumption, given that it takes a struct inode rather than a struct file.
> >
> > If the file is *not* guaranteed to have been opened, then it would be necessary
> > to make fsverity_get_digest() call ensure_verity_info() to set up the
> > fsverity_info.
>
> Yes, fsverity_get_digest() is called as a result of a syscall - open,
> execve, mmap, etc.
> Refer to the LSM hooks security_bprm_check() and security_mmap_file().
> ima_file_check() is called directly in do_open().

stat() is a syscall too, so the question is not whether this is being called as
a result of a syscall, but rather whether it's only being called while the file
is open (or at least previously opened). Is the answer to that "yes"?

- Eric

2022-02-24 02:14:20

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the integrity protected file digest

On Wed, 2022-02-23 at 15:59 -0800, Eric Biggers wrote:
> On Fri, Feb 11, 2022 at 04:43:05PM -0500, Mimi Zohar wrote:
> > +/**
> > + * fsverity_get_digest() - get a verity file's digest
> > + * @inode: inode to get digest of
> > + * @digest: (out) pointer to the digest
> > + * @alg: (out) pointer to the hash algorithm enumeration
> > + *
> > + * Return the file hash algorithm and digest of an fsverity protected file.
> > + *
> > + * Return: 0 on success, -errno on failure
> > + */
> > +int fsverity_get_digest(struct inode *inode,
> > + u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> > + enum hash_algo *alg)
> > +{
> > + const struct fsverity_info *vi;
> > + const struct fsverity_hash_alg *hash_alg;
> > + int i;
> > +
> > + vi = fsverity_get_info(inode);
> > + if (!vi)
> > + return -ENODATA; /* not a verity file */
>
> Sorry for the slow reviews; I'm taking a look again now. One question about
> something I missed earlier: is the file guaranteed to have been opened before
> this is called? fsverity_get_info() only returns a non-NULL value if the file
> has been opened at least once since the inode has been loaded into memory. If
> the inode has just been loaded into memory without being opened, for example due
> to a call to stat(), then fsverity_get_info() will return NULL.
>
> If the file is guaranteed to have been opened, then the code is fine, but the
> comment for fsverity_get_digest() perhaps should be updated to mention this
> assumption, given that it takes a struct inode rather than a struct file.
>
> If the file is *not* guaranteed to have been opened, then it would be necessary
> to make fsverity_get_digest() call ensure_verity_info() to set up the
> fsverity_info.

Yes, fsverity_get_digest() is called as a result of a syscall - open,
execve, mmap, etc.
Refer to the LSM hooks security_bprm_check() and security_mmap_file().
ima_file_check() is called directly in do_open().

Mimi

2022-02-24 02:21:17

by Mimi Zohar

[permalink] [raw]
Subject: Re: [PATCH v5 3/8] fs-verity: define a function to return the integrity protected file digest

On Wed, 2022-02-23 at 17:26 -0800, Eric Biggers wrote:
> On Wed, Feb 23, 2022 at 08:21:01PM -0500, Mimi Zohar wrote:
> > On Wed, 2022-02-23 at 15:59 -0800, Eric Biggers wrote:
> > > On Fri, Feb 11, 2022 at 04:43:05PM -0500, Mimi Zohar wrote:
> > > > +/**
> > > > + * fsverity_get_digest() - get a verity file's digest
> > > > + * @inode: inode to get digest of
> > > > + * @digest: (out) pointer to the digest
> > > > + * @alg: (out) pointer to the hash algorithm enumeration
> > > > + *
> > > > + * Return the file hash algorithm and digest of an fsverity protected file.
> > > > + *
> > > > + * Return: 0 on success, -errno on failure
> > > > + */
> > > > +int fsverity_get_digest(struct inode *inode,
> > > > + u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> > > > + enum hash_algo *alg)
> > > > +{
> > > > + const struct fsverity_info *vi;
> > > > + const struct fsverity_hash_alg *hash_alg;
> > > > + int i;
> > > > +
> > > > + vi = fsverity_get_info(inode);
> > > > + if (!vi)
> > > > + return -ENODATA; /* not a verity file */
> > >
> > > Sorry for the slow reviews; I'm taking a look again now. One question about
> > > something I missed earlier: is the file guaranteed to have been opened before
> > > this is called? fsverity_get_info() only returns a non-NULL value if the file
> > > has been opened at least once since the inode has been loaded into memory. If
> > > the inode has just been loaded into memory without being opened, for example due
> > > to a call to stat(), then fsverity_get_info() will return NULL.
> > >
> > > If the file is guaranteed to have been opened, then the code is fine, but the
> > > comment for fsverity_get_digest() perhaps should be updated to mention this
> > > assumption, given that it takes a struct inode rather than a struct file.
> > >
> > > If the file is *not* guaranteed to have been opened, then it would be necessary
> > > to make fsverity_get_digest() call ensure_verity_info() to set up the
> > > fsverity_info.
> >
> > Yes, fsverity_get_digest() is called as a result of a syscall - open,
> > execve, mmap, etc.
> > Refer to the LSM hooks security_bprm_check() and security_mmap_file().
> > ima_file_check() is called directly in do_open().
>
> stat() is a syscall too, so the question is not whether this is being called as
> a result of a syscall, but rather whether it's only being called while the file
> is open (or at least previously opened). Is the answer to that "yes"?

yes