Message ID | 20220211214310.119257-4-zohar@linux.ibm.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | ima: support fs-verity digests and signatures | expand |
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
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
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
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
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)