[PATCH v2 0/6] ima: support fs-verity digests and signatures

[PATCH v2 0/6] ima: support fs-verity digests and signatures

[Mimi Zohar]

Support for including fs-verity file digests and signatures in the IMA
measurement list as well as verifying the fs-verity file digest based
signatures, all based on IMA policy rules, was discussed from the
beginning, prior to fs-verity being upstreamed[1,2].

Support including fs-verity file digests in the 'd-ng' template field
based on a new policy rule option named 'digest_type=hash|verity'.
Also support verifying fs-verity file digest based signatures based on
policy.

A new template field named 'd-type' as well as a new template named
'ima-ngv2' are defined to differentiate betweeen file hashes and fs-verity
file digests, when file signatures are not included in the IMA measurement
list.

[1] https://events19.linuxfoundation.org/wp-content/uploads/2017/11/fs-verify_Mike-Halcrow_Eric-Biggers.pdf
[2] Documentation/filesystems/fsverity.rst

Changelog v2:
- Addressed Eric Bigger's comments: sign the hash of fsverity's digest
  and the digest's metadata, use match_string, use preferred function
  name fsverity_get_digest(), support including unsigned fs-verity's
  digests in the IMA measurement list.
- Remove signatures requirement for including fs-verity's file digests in
  the 'd-ng' field of the measurement list.

Changelog v1:
- Updated both fsverity and IMA documentation.
- Addressed both Eric Bigger's and Lakshmi's comments.

Mimi Zohar (6):
  ima: rename IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS
  fs-verity: define a function to return the integrity protected file
    digest
  ima: define a new template field 'd-type' and a new template
    'ima-ngv2'
  ima: include fsverity's file digests in the IMA measurement list
  ima: support fs-verity file digest based signatures
  fsverity: update the documentation

 Documentation/ABI/testing/ima_policy      | 17 +++++
 Documentation/filesystems/fsverity.rst    | 22 +++---
 Documentation/security/IMA-templates.rst  | 10 ++-
 fs/verity/Kconfig                         |  1 +
 fs/verity/fsverity_private.h              |  7 --
 fs/verity/measure.c                       | 40 +++++++++++
 include/linux/fsverity.h                  | 18 +++++
 include/uapi/linux/ima.h                  | 26 ++++++++
 security/integrity/ima/ima_api.c          | 29 +++++++-
 security/integrity/ima/ima_appraise.c     | 81 +++++++++++++++++++++++
 security/integrity/ima/ima_main.c         |  2 +-
 security/integrity/ima/ima_policy.c       | 40 ++++++++++-
 security/integrity/ima/ima_template.c     |  3 +
 security/integrity/ima/ima_template_lib.c | 23 ++++++-
 security/integrity/ima/ima_template_lib.h |  2 +
 security/integrity/integrity.h            |  7 +-
 16 files changed, 302 insertions(+), 26 deletions(-)
 create mode 100644 include/uapi/linux/ima.h

-- 
2.27.0

[PATCH v2 1/6] ima: rename IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS

[Mimi Zohar]

Simple policy rule options, such as fowner, uid, or euid, can be checked
immediately, while other policy rule options, such as requiring a file
signature, need to be deferred.

The 'flags' field in the integrity_iint_cache struct contains the policy
action', 'subaction', and non action/subaction.

action: measure/measured, appraise/appraised, (collect)/collected,
        audit/audited
subaction: appraise status for each hook (e.g. file, mmap, bprm, read,
        creds)
non action/subaction: deferred policy rule options and state

Rename the IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_main.c   | 2 +-
 security/integrity/ima/ima_policy.c | 2 +-
 security/integrity/integrity.h      | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 465865412100..e51ddc8d84ee 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -263,7 +263,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 		/* reset appraisal flags if ima_inode_post_setattr was called */
 		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
 				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
-				 IMA_ACTION_FLAGS);
+				 IMA_NONACTION_FLAGS);
 
 	/*
 	 * Re-evaulate the file if either the xattr has changed or the
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 320ca80aacab..a1df015934bc 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -712,7 +712,7 @@ int ima_match_policy(struct user_namespace *mnt_userns, struct inode *inode,
 				     func, mask, func_data))
 			continue;
 
-		action |= entry->flags & IMA_ACTION_FLAGS;
+		action |= entry->flags & IMA_NONACTION_FLAGS;
 
 		action |= entry->action & IMA_DO_MASK;
 		if (entry->action & IMA_APPRAISE) {
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 547425c20e11..d045dccd415a 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -30,8 +30,8 @@
 #define IMA_HASH		0x00000100
 #define IMA_HASHED		0x00000200
 
-/* iint cache flags */
-#define IMA_ACTION_FLAGS	0xff000000
+/* iint policy rule cache flags */
+#define IMA_NONACTION_FLAGS	0xff000000
 #define IMA_DIGSIG_REQUIRED	0x01000000
 #define IMA_PERMIT_DIRECTIO	0x02000000
 #define IMA_NEW_FILE		0x04000000
-- 
2.27.0

[PATCH v2 2/6] fs-verity: define a function to return the integrity protected file digest

[Mimi Zohar]

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

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 fs/verity/Kconfig            |  1 +
 fs/verity/fsverity_private.h |  7 -------
 fs/verity/measure.c          | 40 ++++++++++++++++++++++++++++++++++++
 include/linux/fsverity.h     | 18 ++++++++++++++++
 4 files changed, 59 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..52906b2e5811 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -57,3 +57,43 @@ 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);
+	*alg = HASH_ALGO__LAST;
+
+	/* convert hash algorithm to hash_algo_name */
+	i = match_string(hash_algo_name, HASH_ALGO__LAST, hash_alg->name);
+	if (i < 0)
+		return -EINVAL;
+	*alg = i;
+
+	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

Re: [PATCH v2 2/6] fs-verity: define a function to return the integrity protected file digest

[Vitaly Chikunov]

Mimi,

On Sun, Jan 09, 2022 at 01:55:13PM -0500, Mimi Zohar wrote:
> Define a function named fsverity_get_digest() to return the verity file
> digest and the associated hash algorithm (enum hash_algo).
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  fs/verity/Kconfig            |  1 +
>  fs/verity/fsverity_private.h |  7 -------
>  fs/verity/measure.c          | 40 ++++++++++++++++++++++++++++++++++++
>  include/linux/fsverity.h     | 18 ++++++++++++++++
>  4 files changed, 59 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> index f0d7b30c62db..52906b2e5811 100644
> --- a/fs/verity/measure.c
> +++ b/fs/verity/measure.c
> @@ -57,3 +57,43 @@ 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);
> +	*alg = HASH_ALGO__LAST;

Perhaps this line is redundant (*alg is overwritten later) and would
needlessly mangle output value in case of -EINVAL while it's being
untouched on -ENODATA.

Thanks,

> +
> +	/* convert hash algorithm to hash_algo_name */
> +	i = match_string(hash_algo_name, HASH_ALGO__LAST, hash_alg->name);
> +	if (i < 0)
> +		return -EINVAL;
> +	*alg = i;
> +
> +	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;
> +}

Re: [PATCH v2 2/6] fs-verity: define a function to return the integrity protected file digest

[Mimi Zohar]

Hi Vitaly,

On Mon, 2022-01-10 at 03:47 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Sun, Jan 09, 2022 at 01:55:13PM -0500, Mimi Zohar wrote:
> > Define a function named fsverity_get_digest() to return the verity file
> > digest and the associated hash algorithm (enum hash_algo).
> > 
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> >  fs/verity/Kconfig            |  1 +
> >  fs/verity/fsverity_private.h |  7 -------
> >  fs/verity/measure.c          | 40 ++++++++++++++++++++++++++++++++++++
> >  include/linux/fsverity.h     | 18 ++++++++++++++++
> >  4 files changed, 59 insertions(+), 7 deletions(-)
> > 
> > diff --git a/fs/verity/measure.c b/fs/verity/measure.c
> > index f0d7b30c62db..52906b2e5811 100644
> > --- a/fs/verity/measure.c
> > +++ b/fs/verity/measure.c
> > @@ -57,3 +57,43 @@ 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);
> > +	*alg = HASH_ALGO__LAST;
> 
> Perhaps this line is redundant (*alg is overwritten later) and would
> needlessly mangle output value in case of -EINVAL while it's being
> untouched on -ENODATA.
> 

Thanks!

Mimi

> 
> > +
> > +	/* convert hash algorithm to hash_algo_name */
> > +	i = match_string(hash_algo_name, HASH_ALGO__LAST, hash_alg->name);
> > +	if (i < 0)
> > +		return -EINVAL;
> > +	*alg = i;
> > +
> > +	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;
> > +}

Re: [PATCH v2 2/6] fs-verity: define a function to return the integrity protected file digest

[Eric Biggers]

On Sun, Jan 09, 2022 at 01:55:13PM -0500, Mimi Zohar wrote:
> Define a function named fsverity_get_digest() to return the verity file
> digest and the associated hash algorithm (enum hash_algo).
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---

Acked-by: Eric Biggers <ebiggers@google.com>

> +int fsverity_get_digest(struct inode *inode,
> +			    u8 digest[FS_VERITY_MAX_DIGEST_SIZE],
> +			    enum hash_algo *alg)

There's some weird indentation here.

- Eric

[PATCH v2 3/6] ima: define a new template field 'd-type' and a new template 'ima-ngv2'

[Mimi Zohar]

In preparation to differentiate between regular file hashes and
fs-verity's file digests, define a new template field named 'd-type'.
Define and include the new 'd-type' field in the new template named
'ima-ngv2'.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 security/integrity/ima/ima_template.c     |  3 +++
 security/integrity/ima/ima_template_lib.c | 13 +++++++++++++
 security/integrity/ima/ima_template_lib.h |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 694560396be0..9d8253c6c52c 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -19,6 +19,7 @@ enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME,
 static struct ima_template_desc builtin_templates[] = {
 	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
 	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
+	{.name = "ima-ngv2", .fmt = "d-ng|n-ng|d-type"},
 	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
 	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
@@ -39,6 +40,8 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_digest_ng},
 	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
 	 .field_show = ima_show_template_string},
+	{.field_id = "d-type", .field_init = ima_eventdigest_type_init,
+	 .field_show = ima_show_template_string},
 	{.field_id = "sig", .field_init = ima_eventsig_init,
 	 .field_show = ima_show_template_sig},
 	{.field_id = "buf", .field_init = ima_eventbuf_init,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index ca017cae73eb..fb23bde297f1 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -386,6 +386,19 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
 					   hash_algo, field_data);
 }
 
+/*
+ * This function writes the digest type of an event.
+ */
+int ima_eventdigest_type_init(struct ima_event_data *event_data,
+			      struct ima_field_data *field_data)
+{
+	static const char * const digest_type[] = {"hash"};
+
+	return ima_write_template_field_data(digest_type[0],
+					     strlen(digest_type[0]),
+					     DATA_FMT_STRING, field_data);
+}
+
 /*
  * This function writes the digest of the file which is expected to match the
  * digest contained in the file's appended signature.
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index c71f1de95753..539a5e354925 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -38,6 +38,8 @@ int ima_eventname_init(struct ima_event_data *event_data,
 		       struct ima_field_data *field_data);
 int ima_eventdigest_ng_init(struct ima_event_data *event_data,
 			    struct ima_field_data *field_data);
+int ima_eventdigest_type_init(struct ima_event_data *event_data,
+			      struct ima_field_data *field_data);
 int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
 				struct ima_field_data *field_data);
 int ima_eventname_ng_init(struct ima_event_data *event_data,
-- 
2.27.0

[PATCH v2 4/6] ima: include fsverity's file digests in the IMA measurement list

[Mimi Zohar]

Allow fsverity's file digests to be included in the IMA measurement list
based on policy.

Define a new measurement policy rule option named 'digest_type=' to
allow fsverity file digests to be included in the measurement list
in the d-ng field.

Including the 'd-type' template field is recommended for unsigned
fs-verity digests to distinguish between d-ng digest types.  The
following policy rule, for example, specifies the new 'ima-ngv2'
template.

measure func=FILE_CHECK digest_type=hash|verity template=ima-ngv2

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
Comment:
- To avoid unnecessary patch churn for the proposed IMA namespacing
patch set, don't increase the iint->flags size, but include an
additional bit in the IMA_NONACTION_FLAGS.


 Documentation/ABI/testing/ima_policy      |  7 +++++
 Documentation/security/IMA-templates.rst  |  6 ++++
 security/integrity/ima/ima_api.c          | 29 +++++++++++++++--
 security/integrity/ima/ima_policy.c       | 38 ++++++++++++++++++++++-
 security/integrity/ima/ima_template_lib.c |  9 +++++-
 security/integrity/integrity.h            |  4 ++-
 6 files changed, 88 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 839fab811b18..444bb7ccbe03 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -51,6 +51,7 @@ Description:
 			appraise_flag:= [check_blacklist]
 			Currently, blacklist check is only for files signed with appended
 			signature.
+			digest_type:= [hash|verity]
 			keyrings:= list of keyrings
 			(eg, .builtin_trusted_keys|.ima). Only valid
 			when action is "measure" and func is KEY_CHECK.
@@ -149,3 +150,9 @@ Description:
 		security.ima xattr of a file:
 
 			appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
+
+		Example of measure rule allowing fs-verity's digests on a
+		particular filesystem with indication of type of digest.
+
+			measure func=FILE_CHECK digest_type=hash|verity \
+				fsuuid=... template=ima-ngv2
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 1a91d92950a7..5e31513e8ec4 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -69,6 +69,7 @@ descriptors by adding their identifier to the format string
    algorithm (field format: [<hash algo>:]digest, where the digest
    prefix is shown only if the hash algorithm is not SHA1 or MD5);
  - 'd-modsig': the digest of the event without the appended modsig;
+ - 'd-type': the type of file digest (e.g. hash, verity[1]);
  - 'n-ng': the name of the event, without size limitations;
  - 'sig': the file signature, or the EVM portable signature if the file
    signature is not found;
@@ -106,3 +107,8 @@ currently the following methods are supported:
    the ``ima_template=`` parameter;
  - register a new template descriptor with custom format through the kernel
    command line parameter ``ima_template_fmt=``.
+
+
+References
+==========
+[1] Documentation/filesystems/fsverity.rst
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index a64fb0130b01..8760c4874f7d 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -14,6 +14,7 @@
 #include <linux/xattr.h>
 #include <linux/evm.h>
 #include <linux/iversion.h>
+#include <linux/fsverity.h>
 
 #include "ima.h"
 
@@ -200,6 +201,23 @@ int ima_get_action(struct user_namespace *mnt_userns, struct inode *inode,
 				allowed_algos);
 }
 
+static int ima_get_verity_digest(struct integrity_iint_cache *iint,
+				 struct ima_digest_data *hash)
+{
+	u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
+	enum hash_algo verity_alg;
+	int rc;
+
+	rc = fsverity_get_digest(iint->inode, verity_digest, &verity_alg);
+	if (rc)
+		return -EINVAL;
+	if (hash->algo != verity_alg)
+		return -EINVAL;
+	hash->length = hash_digest_size[verity_alg];
+	memcpy(hash->digest, verity_digest, hash->length);
+	return 0;
+}
+
 /*
  * ima_collect_measurement - collect file measurement
  *
@@ -248,10 +266,17 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	/* Initialize hash digest to 0's in case of failure */
 	memset(&hash.digest, 0, sizeof(hash.digest));
 
-	if (buf)
+	if (buf) {
 		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
-	else
+	} else if (iint->flags & IMA_VERITY_ALLOWED) {
+		result = ima_get_verity_digest(iint, &hash.hdr);
+		if (result < 0)
+			result = ima_calc_file_hash(file, &hash.hdr);
+		else
+			iint->flags |= IMA_VERITY_DIGEST;
+	} else {
 		result = ima_calc_file_hash(file, &hash.hdr);
+	}
 
 	if (result && result != -EBADF && result != -EINVAL)
 		goto out;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a1df015934bc..29aa7a64b834 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1024,6 +1024,7 @@ enum policy_opt {
 	Opt_fowner_gt, Opt_fgroup_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_gid_lt, Opt_egid_lt,
 	Opt_fowner_lt, Opt_fgroup_lt,
+	Opt_digest_type,
 	Opt_appraise_type, Opt_appraise_flag, Opt_appraise_algos,
 	Opt_permit_directio, Opt_pcr, Opt_template, Opt_keyrings,
 	Opt_label, Opt_err
@@ -1066,6 +1067,7 @@ static const match_table_t policy_tokens = {
 	{Opt_egid_lt, "egid<%s"},
 	{Opt_fowner_lt, "fowner<%s"},
 	{Opt_fgroup_lt, "fgroup<%s"},
+	{Opt_digest_type, "digest_type=%s"},
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_appraise_flag, "appraise_flag=%s"},
 	{Opt_appraise_algos, "appraise_algos=%s"},
@@ -1173,6 +1175,21 @@ static void check_template_modsig(const struct ima_template_desc *template)
 #undef MSG
 }
 
+/*
+ * Make sure the policy rule and template format are in sync.
+ */
+static void check_template_field(const struct ima_template_desc *template,
+				 const char *field, const char *msg)
+{
+	int i;
+
+	for (i = 0; i < template->num_fields; i++)
+		if (!strcmp(template->fields[i]->field_id, field))
+			return;
+
+	pr_notice_once("%s", msg);
+}
+
 static bool ima_validate_rule(struct ima_rule_entry *entry)
 {
 	/* Ensure that the action is set and is compatible with the flags */
@@ -1215,7 +1232,8 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
 				     IMA_FSNAME | IMA_GID | IMA_EGID |
 				     IMA_FGROUP | IMA_DIGSIG_REQUIRED |
-				     IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS))
+				     IMA_PERMIT_DIRECTIO | IMA_VALIDATE_ALGOS |
+				     IMA_VERITY_ALLOWED))
 			return false;
 
 		break;
@@ -1708,6 +1726,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 						   LSM_SUBJ_TYPE,
 						   AUDIT_SUBJ_TYPE);
 			break;
+		case Opt_digest_type:
+			ima_log_string(ab, "digest_type", args[0].from);
+			if ((strcmp(args[0].from, "hash|verity")) == 0)
+				entry->flags |= IMA_VERITY_ALLOWED;
+			else
+				result = -EINVAL;
+			break;
 		case Opt_appraise_type:
 			ima_log_string(ab, "appraise_type", args[0].from);
 			if ((strcmp(args[0].from, "imasig")) == 0)
@@ -1798,6 +1823,15 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 		check_template_modsig(template_desc);
 	}
 
+	/* d-type template field recommended for unsigned fs-verity digests */
+	if (!result && entry->action == MEASURE &&
+	    entry->flags & IMA_VERITY_ALLOWED) {
+		template_desc = entry->template ? entry->template :
+						  ima_template_desc_current();
+		check_template_field(template_desc, "d-type",
+				     "verity rules should include d-type");
+	}
+
 	audit_log_format(ab, "res=%d", !result);
 	audit_log_end(ab);
 	return result;
@@ -2147,6 +2181,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 		else
 			seq_puts(m, "appraise_type=imasig ");
 	}
+	if (entry->flags & IMA_VERITY_ALLOWED)
+		seq_puts(m, "digest_type=hash|verity ");
 	if (entry->flags & IMA_CHECK_BLACKLIST)
 		seq_puts(m, "appraise_flag=check_blacklist ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index fb23bde297f1..1c0cea2b805f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -392,7 +392,14 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
 int ima_eventdigest_type_init(struct ima_event_data *event_data,
 			      struct ima_field_data *field_data)
 {
-	static const char * const digest_type[] = {"hash"};
+	static const char * const digest_type[] = {"hash", "verity"};
+
+	if (event_data->iint->flags & IMA_VERITY_DIGEST) {
+		return ima_write_template_field_data(digest_type[1],
+						     strlen(digest_type[1]),
+						     DATA_FMT_STRING,
+						     field_data);
+	}
 
 	return ima_write_template_field_data(digest_type[0],
 					     strlen(digest_type[0]),
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index d045dccd415a..e7ac1086d1d9 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -31,7 +31,7 @@
 #define IMA_HASHED		0x00000200
 
 /* iint policy rule cache flags */
-#define IMA_NONACTION_FLAGS	0xff000000
+#define IMA_NONACTION_FLAGS	0xff800000
 #define IMA_DIGSIG_REQUIRED	0x01000000
 #define IMA_PERMIT_DIRECTIO	0x02000000
 #define IMA_NEW_FILE		0x04000000
@@ -39,6 +39,8 @@
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
 #define IMA_MODSIG_ALLOWED	0x20000000
 #define IMA_CHECK_BLACKLIST	0x40000000
+#define IMA_VERITY_ALLOWED	0x80000000
+#define IMA_VERITY_DIGEST	0x00800000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.27.0

[PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Mimi Zohar]

Instead of calculating a file hash and verifying the signature stored
in the security.ima xattr against the calculated file hash, verify the
signature based on a hash of fs-verity's file digest and the digest's
metadata.

To differentiate between a regular file hash and an fs-verity file digest
based signature stored as security.ima xattr, define a new signature type
named IMA_VERITY_DIGSIG.

The hash format of fs-verity's file digest and the digest's metadata to
be signed is defined as a structure named "ima_tbs_hash".

Update the 'ima-sig' template field to display the new fs-verity signature
type as well.

For example:
  appraise func=BPRM_CHECK digest_type=hash|verity

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy      | 10 +++
 Documentation/security/IMA-templates.rst  |  4 +-
 include/uapi/linux/ima.h                  | 26 ++++++++
 security/integrity/ima/ima_appraise.c     | 81 +++++++++++++++++++++++
 security/integrity/ima/ima_template_lib.c |  3 +-
 security/integrity/integrity.h            |  1 +
 6 files changed, 122 insertions(+), 3 deletions(-)
 create mode 100644 include/uapi/linux/ima.h

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 444bb7ccbe03..fadf90dde289 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -151,6 +151,16 @@ Description:
 
 			appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
 
+		Example of measure and appraise rules allowing fs-verity
+		signed digests on a particular filesystem identified by
+		it's fsuuid:
+
+			measure func=BPRM_CHECK digest_type=hash|verity \
+				fsuuid=... template=ima-sig
+			appraise func=BPRM_CHECK digest_type=hash|verity \
+				fsuuid=...
+
+
 		Example of measure rule allowing fs-verity's digests on a
 		particular filesystem with indication of type of digest.
 
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 5e31513e8ec4..390936810ebc 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -71,8 +71,8 @@ descriptors by adding their identifier to the format string
  - 'd-modsig': the digest of the event without the appended modsig;
  - 'd-type': the type of file digest (e.g. hash, verity[1]);
  - 'n-ng': the name of the event, without size limitations;
- - 'sig': the file signature, or the EVM portable signature if the file
-   signature is not found;
+ - 'sig': the file signature, based on either the file's/fsverity's digest[1],
+   or the EVM portable signature if the file signature is not found;
  - 'modsig' the appended file signature;
  - 'buf': the buffer data that was used to generate the hash without size limitations;
  - 'evmsig': the EVM portable signature;
diff --git a/include/uapi/linux/ima.h b/include/uapi/linux/ima.h
new file mode 100644
index 000000000000..6a2a68fc0fad
--- /dev/null
+++ b/include/uapi/linux/ima.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
+/*
+ * IMA user API
+ *
+ */
+#ifndef _UAPI_LINUX_IMA_H
+#define _UAPI_LINUX_IMA_H
+
+#include <linux/types.h>
+
+/*
+ * The hash format of fs-verity's file digest and other file metadata
+ * to be signed.  The resulting signature is stored as a security.ima
+ * xattr.
+ *
+ * "type" is defined as IMA_VERITY_DIGSIG
+ * "algo" is the hash_algo enum of fs-verity's file digest
+ * (e.g. HASH_ALGO_SHA256, HASH_ALGO_SHA512).
+ */
+struct ima_tbs_hash {
+	__u8 type;        /* xattr type [enum evm_ima_xattr_type] */
+	__u8 algo;        /* Digest algorithm [enum hash_algo] */
+	__u8 digest[];    /* fs-verity digest */
+};
+
+#endif /* _UAPI_LINUX_IMA_H */
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index dbba51583e7c..4e092c189ed0 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -13,7 +13,10 @@
 #include <linux/magic.h>
 #include <linux/ima.h>
 #include <linux/evm.h>
+#include <linux/fsverity.h>
 #include <keys/system_keyring.h>
+#include <uapi/linux/fsverity.h>
+#include <uapi/linux/ima.h>
 
 #include "ima.h"
 
@@ -183,6 +186,8 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
 		return ima_hash_algo;
 
 	switch (xattr_value->type) {
+	case IMA_VERITY_DIGSIG:
+		fallthrough;
 	case EVM_IMA_XATTR_DIGSIG:
 		sig = (typeof(sig))xattr_value;
 		if (sig->version != 2 || xattr_len <= sizeof(*sig)
@@ -225,6 +230,47 @@ int ima_read_xattr(struct dentry *dentry,
 	return ret;
 }
 
+/*
+ * calc_tbs_hash - calculate hash of a digest and digest metadata
+ * @type: signature type [IMA_VERITY_DIGSIG]
+ * @algo: hash algorithm [enum hash_algo]
+ * @digest: pointer to the digest to be hashed
+ * @hash: (out) pointer to the hash
+ *
+ * The IMA signature is a signature over the hash of fs-verity's file digest
+ * with other digest metadata, not just fs-verity's file digest. Calculate
+ * the to be signed hash.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int calc_tbs_hash(enum evm_ima_xattr_type xattr_type,
+			 enum hash_algo algo, const u8 *digest,
+			 struct ima_digest_data *hash)
+{
+	struct ima_tbs_hash *tbs_h;
+	int rc = 0;
+
+	if (xattr_type != IMA_VERITY_DIGSIG)
+		return -EINVAL;
+
+	tbs_h = kzalloc(sizeof(*tbs_h) + hash_digest_size[algo], GFP_KERNEL);
+	if (!tbs_h)
+		return -ENOMEM;
+
+	tbs_h->type = xattr_type;
+	tbs_h->algo = algo;
+	memcpy(tbs_h->digest, digest, hash_digest_size[algo]);
+
+	hash->algo = algo;
+	hash->length = hash_digest_size[algo];
+
+	rc = ima_calc_buffer_hash(tbs_h,
+				  sizeof(*tbs_h) + hash_digest_size[algo],
+				  hash);
+	kfree(tbs_h);
+	return rc;
+}
+
 /*
  * xattr_verify - verify xattr digest or signature
  *
@@ -236,7 +282,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 			struct evm_ima_xattr_data *xattr_value, int xattr_len,
 			enum integrity_status *status, const char **cause)
 {
+	struct ima_digest_data *hash = NULL;
 	int rc = -EINVAL, hash_start = 0;
+	u8 algo;  /* Digest algorithm [enum hash_algo] */
 
 	switch (xattr_value->type) {
 	case IMA_XATTR_DIGEST_NG:
@@ -271,6 +319,38 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 			break;
 		}
 		*status = INTEGRITY_PASS;
+		break;
+	case IMA_VERITY_DIGSIG:
+		set_bit(IMA_DIGSIG, &iint->atomic_flags);
+
+		algo = iint->ima_hash->algo;
+		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
+			       GFP_KERNEL);
+		if (!hash) {
+			*cause = "verity-hashing-error";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+
+		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
+				   iint->ima_hash->digest, hash);
+		if (rc) {
+			*cause = "verity-hashing-error";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+
+		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+					     (const char *)xattr_value,
+					     xattr_len, hash->digest,
+					     hash->length);
+		if (rc) {
+			*cause = "invalid-verity-signature";
+			*status = INTEGRITY_FAIL;
+		} else {
+			*status = INTEGRITY_PASS;
+		}
+
 		break;
 	case EVM_IMA_XATTR_DIGSIG:
 		set_bit(IMA_DIGSIG, &iint->atomic_flags);
@@ -303,6 +383,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 		break;
 	}
 
+	kfree(hash);
 	return rc;
 }
 
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 1c0cea2b805f..31a14943e459 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -498,7 +498,8 @@ int ima_eventsig_init(struct ima_event_data *event_data,
 {
 	struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
 
-	if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
+	if ((!xattr_value) ||
+	     !(xattr_value->type & (EVM_IMA_XATTR_DIGSIG | IMA_VERITY_DIGSIG)))
 		return ima_eventevmsig_init(event_data, field_data);
 
 	return ima_write_template_field_data(xattr_value, event_data->xattr_len,
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e7ac1086d1d9..51124708c072 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -79,6 +79,7 @@ enum evm_ima_xattr_type {
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
 	EVM_XATTR_PORTABLE_DIGSIG,
+	IMA_VERITY_DIGSIG,
 	IMA_XATTR_LAST
 };
 
-- 
2.27.0

Re: [PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Vitaly Chikunov]

Mimi,

On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote:
> Instead of calculating a file hash and verifying the signature stored
> in the security.ima xattr against the calculated file hash, verify the
> signature based on a hash of fs-verity's file digest and the digest's
> metadata.
> 
> To differentiate between a regular file hash and an fs-verity file digest
> based signature stored as security.ima xattr, define a new signature type
> named IMA_VERITY_DIGSIG.
> 
> The hash format of fs-verity's file digest and the digest's metadata to
> be signed is defined as a structure named "ima_tbs_hash".
> 
> Update the 'ima-sig' template field to display the new fs-verity signature
> type as well.
> 
> For example:
>   appraise func=BPRM_CHECK digest_type=hash|verity
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>  Documentation/ABI/testing/ima_policy      | 10 +++
>  Documentation/security/IMA-templates.rst  |  4 +-
>  include/uapi/linux/ima.h                  | 26 ++++++++
>  security/integrity/ima/ima_appraise.c     | 81 +++++++++++++++++++++++
>  security/integrity/ima/ima_template_lib.c |  3 +-
>  security/integrity/integrity.h            |  1 +
>  6 files changed, 122 insertions(+), 3 deletions(-)
>  create mode 100644 include/uapi/linux/ima.h
> 
> diff --git a/include/uapi/linux/ima.h b/include/uapi/linux/ima.h
> new file mode 100644
> index 000000000000..6a2a68fc0fad
> --- /dev/null
> +++ b/include/uapi/linux/ima.h
> @@ -0,0 +1,26 @@
> +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> +/*
> + * IMA user API
> + *
> + */
> +#ifndef _UAPI_LINUX_IMA_H
> +#define _UAPI_LINUX_IMA_H
> +
> +#include <linux/types.h>
> +
> +/*
> + * The hash format of fs-verity's file digest and other file metadata
> + * to be signed.  The resulting signature is stored as a security.ima
> + * xattr.
> + *
> + * "type" is defined as IMA_VERITY_DIGSIG
> + * "algo" is the hash_algo enum of fs-verity's file digest
> + * (e.g. HASH_ALGO_SHA256, HASH_ALGO_SHA512).
> + */
> +struct ima_tbs_hash {
> +	__u8 type;        /* xattr type [enum evm_ima_xattr_type] */
> +	__u8 algo;        /* Digest algorithm [enum hash_algo] */
> +	__u8 digest[];    /* fs-verity digest */

Maximum digest size is known to be FS_VERITY_MAX_DIGEST_SIZE. If it's
allocated here then ima_tbs_hash could be allocated temporarily on stack
instead of kalloc.

> +};
> +
> +#endif /* _UAPI_LINUX_IMA_H */
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index dbba51583e7c..4e092c189ed0 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -13,7 +13,10 @@
>  #include <linux/magic.h>
>  #include <linux/ima.h>
>  #include <linux/evm.h>
> +#include <linux/fsverity.h>
>  #include <keys/system_keyring.h>
> +#include <uapi/linux/fsverity.h>
> +#include <uapi/linux/ima.h>
>  
>  #include "ima.h"
>  
> @@ -183,6 +186,8 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
>  		return ima_hash_algo;
>  
>  	switch (xattr_value->type) {
> +	case IMA_VERITY_DIGSIG:
> +		fallthrough;

I think fallthrough is not needed there, since it's just multiple case's
and no code.

>  	case EVM_IMA_XATTR_DIGSIG:
>  		sig = (typeof(sig))xattr_value;
>  		if (sig->version != 2 || xattr_len <= sizeof(*sig)
> @@ -225,6 +230,47 @@ int ima_read_xattr(struct dentry *dentry,
>  	return ret;
>  }
>  
> +/*
> + * calc_tbs_hash - calculate hash of a digest and digest metadata
> + * @type: signature type [IMA_VERITY_DIGSIG]

Parameter seems renamed, but why it's ever need if it's called once and
ever with IMA_VERITY_DIGSIG? If it's deleted then its value no need to be
checked below.

> + * @algo: hash algorithm [enum hash_algo]
> + * @digest: pointer to the digest to be hashed
> + * @hash: (out) pointer to the hash
> + *
> + * The IMA signature is a signature over the hash of fs-verity's file digest
> + * with other digest metadata, not just fs-verity's file digest. Calculate
> + * the to be signed hash.
> + *
> + * Return 0 on success, error code otherwise.
> + */
> +static int calc_tbs_hash(enum evm_ima_xattr_type xattr_type,
> +			 enum hash_algo algo, const u8 *digest,
> +			 struct ima_digest_data *hash)
> +{
> +	struct ima_tbs_hash *tbs_h;
> +	int rc = 0;
> +
> +	if (xattr_type != IMA_VERITY_DIGSIG)
> +		return -EINVAL;
> +
> +	tbs_h = kzalloc(sizeof(*tbs_h) + hash_digest_size[algo], GFP_KERNEL);
> +	if (!tbs_h)
> +		return -ENOMEM;
> +
> +	tbs_h->type = xattr_type;
> +	tbs_h->algo = algo;
> +	memcpy(tbs_h->digest, digest, hash_digest_size[algo]);
> +
> +	hash->algo = algo;

As I understood all of this - hash algo used in fs-verity and algo used
to hash it here are the same. Ultimate source of which is algo id from
xattr - if fs-verity digest algo differs from xattr's then fs-verity
digest is ignored.

Thanks,

> +	hash->length = hash_digest_size[algo];
> +
> +	rc = ima_calc_buffer_hash(tbs_h,
> +				  sizeof(*tbs_h) + hash_digest_size[algo],
> +				  hash);
> +	kfree(tbs_h);
> +	return rc;
> +}
> +
>  /*
>   * xattr_verify - verify xattr digest or signature
>   *
> @@ -236,7 +282,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
>  			struct evm_ima_xattr_data *xattr_value, int xattr_len,
>  			enum integrity_status *status, const char **cause)
>  {
> +	struct ima_digest_data *hash = NULL;
>  	int rc = -EINVAL, hash_start = 0;
> +	u8 algo;  /* Digest algorithm [enum hash_algo] */
>  
>  	switch (xattr_value->type) {
>  	case IMA_XATTR_DIGEST_NG:
> @@ -271,6 +319,38 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
>  			break;
>  		}
>  		*status = INTEGRITY_PASS;
> +		break;
> +	case IMA_VERITY_DIGSIG:
> +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> +
> +		algo = iint->ima_hash->algo;
> +		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
> +			       GFP_KERNEL);
> +		if (!hash) {
> +			*cause = "verity-hashing-error";
> +			*status = INTEGRITY_FAIL;
> +			break;
> +		}
> +
> +		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
> +				   iint->ima_hash->digest, hash);
> +		if (rc) {
> +			*cause = "verity-hashing-error";
> +			*status = INTEGRITY_FAIL;
> +			break;
> +		}
> +
> +		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> +					     (const char *)xattr_value,
> +					     xattr_len, hash->digest,
> +					     hash->length);
> +		if (rc) {
> +			*cause = "invalid-verity-signature";
> +			*status = INTEGRITY_FAIL;
> +		} else {
> +			*status = INTEGRITY_PASS;
> +		}
> +
>  		break;
>  	case EVM_IMA_XATTR_DIGSIG:
>  		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> @@ -303,6 +383,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
>  		break;
>  	}
>  
> +	kfree(hash);
>  	return rc;
>  }
>  
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 1c0cea2b805f..31a14943e459 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -498,7 +498,8 @@ int ima_eventsig_init(struct ima_event_data *event_data,
>  {
>  	struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
>  
> -	if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
> +	if ((!xattr_value) ||
> +	     !(xattr_value->type & (EVM_IMA_XATTR_DIGSIG | IMA_VERITY_DIGSIG)))
>  		return ima_eventevmsig_init(event_data, field_data);
>  
>  	return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index e7ac1086d1d9..51124708c072 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -79,6 +79,7 @@ enum evm_ima_xattr_type {
>  	EVM_IMA_XATTR_DIGSIG,
>  	IMA_XATTR_DIGEST_NG,
>  	EVM_XATTR_PORTABLE_DIGSIG,
> +	IMA_VERITY_DIGSIG,
>  	IMA_XATTR_LAST
>  };
>  
> -- 
> 2.27.0

Re: [PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Mimi Zohar]

On Mon, 2022-01-10 at 04:24 +0300, Vitaly Chikunov wrote:
> Mimi,
> 
> On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote:
> > Instead of calculating a file hash and verifying the signature stored
> > in the security.ima xattr against the calculated file hash, verify the
> > signature based on a hash of fs-verity's file digest and the digest's
> > metadata.
> > 
> > To differentiate between a regular file hash and an fs-verity file digest
> > based signature stored as security.ima xattr, define a new signature type
> > named IMA_VERITY_DIGSIG.
> > 
> > The hash format of fs-verity's file digest and the digest's metadata to
> > be signed is defined as a structure named "ima_tbs_hash".
> > 
> > Update the 'ima-sig' template field to display the new fs-verity signature
> > type as well.
> > 
> > For example:
> >   appraise func=BPRM_CHECK digest_type=hash|verity
> > 
> > Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> >  Documentation/ABI/testing/ima_policy      | 10 +++
> >  Documentation/security/IMA-templates.rst  |  4 +-
> >  include/uapi/linux/ima.h                  | 26 ++++++++
> >  security/integrity/ima/ima_appraise.c     | 81 +++++++++++++++++++++++
> >  security/integrity/ima/ima_template_lib.c |  3 +-
> >  security/integrity/integrity.h            |  1 +
> >  6 files changed, 122 insertions(+), 3 deletions(-)
> >  create mode 100644 include/uapi/linux/ima.h
> > 
> > diff --git a/include/uapi/linux/ima.h b/include/uapi/linux/ima.h
> > new file mode 100644
> > index 000000000000..6a2a68fc0fad
> > --- /dev/null
> > +++ b/include/uapi/linux/ima.h
> > @@ -0,0 +1,26 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only WITH Linux-syscall-note */
> > +/*
> > + * IMA user API
> > + *
> > + */
> > +#ifndef _UAPI_LINUX_IMA_H
> > +#define _UAPI_LINUX_IMA_H
> > +
> > +#include <linux/types.h>
> > +
> > +/*
> > + * The hash format of fs-verity's file digest and other file metadata
> > + * to be signed.  The resulting signature is stored as a security.ima
> > + * xattr.
> > + *
> > + * "type" is defined as IMA_VERITY_DIGSIG
> > + * "algo" is the hash_algo enum of fs-verity's file digest
> > + * (e.g. HASH_ALGO_SHA256, HASH_ALGO_SHA512).
> > + */
> > +struct ima_tbs_hash {
> > +	__u8 type;        /* xattr type [enum evm_ima_xattr_type] */
> > +	__u8 algo;        /* Digest algorithm [enum hash_algo] */
> > +	__u8 digest[];    /* fs-verity digest */
> 
> Maximum digest size is known to be FS_VERITY_MAX_DIGEST_SIZE. If it's
> allocated here then ima_tbs_hash could be allocated temporarily on stack
> instead of kalloc.

The buffer size to be hashed is currently straight forward
"sizeof(*tbs_h) + hash_digest_size[algo]".   With this change, we would
need to calculate the unused part of FS_VERITY_MAX_DIGEST_SIZE and
subtract it from the struct size.  Perhaps something like:
    "sizeof(*tbs_h) - (FS_VERITY_MAX_DIGEST_SIZE -
hash_digest_size[algo])"

thanks,

Mimi

> 
> > +};
> > +
> > +#endif /* _UAPI_LINUX_IMA_H */
> > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> > index dbba51583e7c..4e092c189ed0 100644
> > --- a/security/integrity/ima/ima_appraise.c
> > +++ b/security/integrity/ima/ima_appraise.c
> > @@ -13,7 +13,10 @@
> >  #include <linux/magic.h>
> >  #include <linux/ima.h>
> >  #include <linux/evm.h>
> > +#include <linux/fsverity.h>
> >  #include <keys/system_keyring.h>
> > +#include <uapi/linux/fsverity.h>
> > +#include <uapi/linux/ima.h>
> >  
> >  #include "ima.h"
> >  
> > @@ -183,6 +186,8 @@ enum hash_algo ima_get_hash_algo(const struct evm_ima_xattr_data *xattr_value,
> >  		return ima_hash_algo;
> >  
> >  	switch (xattr_value->type) {
> > +	case IMA_VERITY_DIGSIG:
> > +		fallthrough;
> 
> I think fallthrough is not needed there, since it's just multiple case's
> and no code.

Thanks.

> 
> >  	case EVM_IMA_XATTR_DIGSIG:
> >  		sig = (typeof(sig))xattr_value;
> >  		if (sig->version != 2 || xattr_len <= sizeof(*sig)
> > @@ -225,6 +230,47 @@ int ima_read_xattr(struct dentry *dentry,
> >  	return ret;
> >  }
> >  
> > +/*
> > + * calc_tbs_hash - calculate hash of a digest and digest metadata
> > + * @type: signature type [IMA_VERITY_DIGSIG]
> 
> Parameter seems renamed, but why it's ever need if it's called once and
> ever with IMA_VERITY_DIGSIG? If it's deleted then its value no need to be
> checked below.

Thanks for catching the comment and function parameter mismatch.   At
the moment calc_tbs_hash() is limited to fs-verity.  True the parameter
isn't needed now, but at some point we probably should add similar
support for regular file hashes.   That will require incrementing the
signature version number for backwards compatability.

> 
> > + * @algo: hash algorithm [enum hash_algo]
> > + * @digest: pointer to the digest to be hashed
> > + * @hash: (out) pointer to the hash
> > + *
> > + * The IMA signature is a signature over the hash of fs-verity's file digest
> > + * with other digest metadata, not just fs-verity's file digest. Calculate
> > + * the to be signed hash.
> > + *
> > + * Return 0 on success, error code otherwise.
> > + */
> > +static int calc_tbs_hash(enum evm_ima_xattr_type xattr_type,
> > +			 enum hash_algo algo, const u8 *digest,
> > +			 struct ima_digest_data *hash)
> > +{
> > +	struct ima_tbs_hash *tbs_h;
> > +	int rc = 0;
> > +
> > +	if (xattr_type != IMA_VERITY_DIGSIG)
> > +		return -EINVAL;
> > +
> > +	tbs_h = kzalloc(sizeof(*tbs_h) + hash_digest_size[algo], GFP_KERNEL);
> > +	if (!tbs_h)
> > +		return -ENOMEM;
> > +
> > +	tbs_h->type = xattr_type;
> > +	tbs_h->algo = algo;
> > +	memcpy(tbs_h->digest, digest, hash_digest_size[algo]);
> > +
> > +	hash->algo = algo;
> 
> As I understood all of this - hash algo used in fs-verity and algo used
> to hash it here are the same. Ultimate source of which is algo id from
> xattr - if fs-verity digest algo differs from xattr's then fs-verity
> digest is ignored.

Right, the assumption is that the fs-verity digest, the tbs hash, and
signature all use the same hash algorithm.

> 
> > +	hash->length = hash_digest_size[algo];
> > +
> > +	rc = ima_calc_buffer_hash(tbs_h,
> > +				  sizeof(*tbs_h) + hash_digest_size[algo],
> > +				  hash);
> > +	kfree(tbs_h);
> > +	return rc;
> > +}
> > +
> >  /*
> >   * xattr_verify - verify xattr digest or signature
> >   *
> > @@ -236,7 +282,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
> >  			struct evm_ima_xattr_data *xattr_value, int xattr_len,
> >  			enum integrity_status *status, const char **cause)
> >  {
> > +	struct ima_digest_data *hash = NULL;
> >  	int rc = -EINVAL, hash_start = 0;
> > +	u8 algo;  /* Digest algorithm [enum hash_algo] */
> >  
> >  	switch (xattr_value->type) {
> >  	case IMA_XATTR_DIGEST_NG:
> > @@ -271,6 +319,38 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
> >  			break;
> >  		}
> >  		*status = INTEGRITY_PASS;
> > +		break;
> > +	case IMA_VERITY_DIGSIG:
> > +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> > +
> > +		algo = iint->ima_hash->algo;
> > +		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
> > +			       GFP_KERNEL);
> > +		if (!hash) {
> > +			*cause = "verity-hashing-error";
> > +			*status = INTEGRITY_FAIL;
> > +			break;
> > +		}
> > +
> > +		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
> > +				   iint->ima_hash->digest, hash);
> > +		if (rc) {
> > +			*cause = "verity-hashing-error";
> > +			*status = INTEGRITY_FAIL;
> > +			break;
> > +		}
> > +
> > +		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> > +					     (const char *)xattr_value,
> > +					     xattr_len, hash->digest,
> > +					     hash->length);
> > +		if (rc) {
> > +			*cause = "invalid-verity-signature";
> > +			*status = INTEGRITY_FAIL;
> > +		} else {
> > +			*status = INTEGRITY_PASS;
> > +		}
> > +
> >  		break;
> >  	case EVM_IMA_XATTR_DIGSIG:
> >  		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> > @@ -303,6 +383,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
> >  		break;
> >  	}
> >  
> > +	kfree(hash);
> >  	return rc;
> >  }
> >  
> > diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> > index 1c0cea2b805f..31a14943e459 100644
> > --- a/security/integrity/ima/ima_template_lib.c
> > +++ b/security/integrity/ima/ima_template_lib.c
> > @@ -498,7 +498,8 @@ int ima_eventsig_init(struct ima_event_data *event_data,
> >  {
> >  	struct evm_ima_xattr_data *xattr_value = event_data->xattr_value;
> >  
> > -	if ((!xattr_value) || (xattr_value->type != EVM_IMA_XATTR_DIGSIG))
> > +	if ((!xattr_value) ||
> > +	     !(xattr_value->type & (EVM_IMA_XATTR_DIGSIG | IMA_VERITY_DIGSIG)))
> >  		return ima_eventevmsig_init(event_data, field_data);
> >  
> >  	return ima_write_template_field_data(xattr_value, event_data->xattr_len,
> > diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> > index e7ac1086d1d9..51124708c072 100644
> > --- a/security/integrity/integrity.h
> > +++ b/security/integrity/integrity.h
> > @@ -79,6 +79,7 @@ enum evm_ima_xattr_type {
> >  	EVM_IMA_XATTR_DIGSIG,
> >  	IMA_XATTR_DIGEST_NG,
> >  	EVM_XATTR_PORTABLE_DIGSIG,
> > +	IMA_VERITY_DIGSIG,
> >  	IMA_XATTR_LAST
> >  };
> >  
> > -- 
> > 2.27.0

Re: [PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Eric Biggers]

On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote:
> +	case IMA_VERITY_DIGSIG:
> +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> +
> +		algo = iint->ima_hash->algo;
> +		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
> +			       GFP_KERNEL);
> +		if (!hash) {
> +			*cause = "verity-hashing-error";
> +			*status = INTEGRITY_FAIL;
> +			break;
> +		}
> +
> +		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
> +				   iint->ima_hash->digest, hash);
> +		if (rc) {
> +			*cause = "verity-hashing-error";
> +			*status = INTEGRITY_FAIL;
> +			break;
> +		}
> +
> +		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> +					     (const char *)xattr_value,
> +					     xattr_len, hash->digest,
> +					     hash->length);

This is still verifying a raw hash value, which is wrong as I've explained
several times.  Yes, you are now hashing the hash algorithm ID together with the
original hash value, but at the end the thing being signed/verified is still a
raw hash value, which is ambigious.

I think I see where the confusion is.  If rsa-pkcs1pad is used, then the
asymmetric algorithm is parameterized by a hash algorithm, and this hash
algorithm's identifier is automatically built-in to the data which is
signed/verified.  And the data being signed/verified is assumed to be a hash
value of the same type.  So in this case, the caller doesn't need to handle
disambiguating raw hashes.

However, asymmetric_verify() also supports ecdsa and ecrdsa signatures.  As far
as I can tell, those do *not* have the hash algorithm identifier built-in to the
data which is signed/verified; they just sign/verify the data given.  That
creates an ambiguity if the hash algorithm identifier is not included.  For
example, someone might have intended to sign the SHA-256 hash
01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b.  However, the
Streebog or SM3 hash
01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b would also pass
the signature check too.  That's wrong; to have a valid cryptosystem, you
mustn't let the adversary choose the crypto algorithms for you.

I'm not sure how this can be reconciled, given the differences between
rsa-pkcs1pad and ecdsa and ecrdsa.  Could you just use the lowest common
denominator and prepend the hash algorithm ID to the hash value, or would that
cause issues with rsa-pkcs1pad?  In any case, to move forward you're going to
need to solve this problem.

- Eric

Re: [PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Stefan Berger]

On 1/10/22 17:45, Eric Biggers wrote:
> On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote:
>> +	case IMA_VERITY_DIGSIG:
>> +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
>> +
>> +		algo = iint->ima_hash->algo;
>> +		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
>> +			       GFP_KERNEL);
>> +		if (!hash) {
>> +			*cause = "verity-hashing-error";
>> +			*status = INTEGRITY_FAIL;
>> +			break;
>> +		}
>> +
>> +		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
>> +				   iint->ima_hash->digest, hash);
>> +		if (rc) {
>> +			*cause = "verity-hashing-error";
>> +			*status = INTEGRITY_FAIL;
>> +			break;
>> +		}
>> +
>> +		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
>> +					     (const char *)xattr_value,
>> +					     xattr_len, hash->digest,
>> +					     hash->length);
> This is still verifying a raw hash value, which is wrong as I've explained
> several times.  Yes, you are now hashing the hash algorithm ID together with the
> original hash value, but at the end the thing being signed/verified is still a
> raw hash value, which is ambigious.
>
> I think I see where the confusion is.  If rsa-pkcs1pad is used, then the
> asymmetric algorithm is parameterized by a hash algorithm, and this hash
> algorithm's identifier is automatically built-in to the data which is
> signed/verified.  And the data being signed/verified is assumed to be a hash
> value of the same type.  So in this case, the caller doesn't need to handle
> disambiguating raw hashes.
>
> However, asymmetric_verify() also supports ecdsa and ecrdsa signatures.  As far
> as I can tell, those do *not* have the hash algorithm identifier built-in to the
> data which is signed/verified; they just sign/verify the data given.  That


The signatures are generated by evmctl by this code here, which works 
for RSA and ECDSA and likely also ECRDSA:

https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/master/tree/src/libimaevm.c#l1036

    if (!EVP_PKEY_sign_init(ctx))
         goto err;
     st = "EVP_get_digestbyname";
     if (!(md = EVP_get_digestbyname(algo)))
         goto err;
     st = "EVP_PKEY_CTX_set_signature_md";
     if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
         goto err;
     st = "EVP_PKEY_sign";
     sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
     if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
         goto err;
     len = (int)sigsize;

As far as I know, these are not raw signatures but generate the OIDs for 
RSA + shaXYZ or ECDSA + shaXYZ (1.2.840.10045.4.1 et al) and prepend 
them to the hash and then sign that.


> creates an ambiguity if the hash algorithm identifier is not included.  For
> example, someone might have intended to sign the SHA-256 hash
> 01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b.  However, the
> Streebog or SM3 hash
> 01ba4719c80b6fe911b091a7c05124b64eeece964e09c058ef8f9805daca546b would also pass
> the signature check too.  That's wrong; to have a valid cryptosystem, you
> mustn't let the adversary choose the crypto algorithms for you.

There's a hash algorithm identifier in the xattr in the header, which is 
prepended to the bytes of the actual signature. This hash algo identifer 
tells IMA which hash to use on the file data so that subsequent 
signature verification with the same hash works. That same hash 
identifier is then again embedded in the signature using the OID and 
thus has to match on the signature verification level.

The effectively double hashed data via calc_tbs_hash() above is not 
good. calc_tbs_hash() is unnecessary.

On the evmctl level the signature should be created from the digest 
retrieved via ioctl() [or similar I suppose] from fsverity on the file 
and fsverity presumably then also says what type of hash this is. So, 
fsverity ioctl response of hash + size of hash and hash_algo become 
input to the evmctl snippet above. On the kernel level the data from 
fsverity_get_digest() should be all it takes to verify against an xattr 
created by evmctl as described.


>
> I'm not sure how this can be reconciled, given the differences between
> rsa-pkcs1pad and ecdsa and ecrdsa.  Could you just use the lowest common
> denominator and prepend the hash algorithm ID to the hash value, or would that
> cause issues with rsa-pkcs1pad?  In any case, to move forward you're going to
> need to solve this problem.
>
> - Eric

Re: [PATCH v2 5/6] ima: support fs-verity file digest based signatures

[Eric Biggers]

On Mon, Jan 10, 2022 at 10:26:23PM -0500, Stefan Berger wrote:
> 
> On 1/10/22 17:45, Eric Biggers wrote:
> > On Sun, Jan 09, 2022 at 01:55:16PM -0500, Mimi Zohar wrote:
> > > +	case IMA_VERITY_DIGSIG:
> > > +		set_bit(IMA_DIGSIG, &iint->atomic_flags);
> > > +
> > > +		algo = iint->ima_hash->algo;
> > > +		hash = kzalloc(sizeof(*hash) + hash_digest_size[algo],
> > > +			       GFP_KERNEL);
> > > +		if (!hash) {
> > > +			*cause = "verity-hashing-error";
> > > +			*status = INTEGRITY_FAIL;
> > > +			break;
> > > +		}
> > > +
> > > +		rc = calc_tbs_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
> > > +				   iint->ima_hash->digest, hash);
> > > +		if (rc) {
> > > +			*cause = "verity-hashing-error";
> > > +			*status = INTEGRITY_FAIL;
> > > +			break;
> > > +		}
> > > +
> > > +		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> > > +					     (const char *)xattr_value,
> > > +					     xattr_len, hash->digest,
> > > +					     hash->length);
> > This is still verifying a raw hash value, which is wrong as I've explained
> > several times.  Yes, you are now hashing the hash algorithm ID together with the
> > original hash value, but at the end the thing being signed/verified is still a
> > raw hash value, which is ambigious.
> > 
> > I think I see where the confusion is.  If rsa-pkcs1pad is used, then the
> > asymmetric algorithm is parameterized by a hash algorithm, and this hash
> > algorithm's identifier is automatically built-in to the data which is
> > signed/verified.  And the data being signed/verified is assumed to be a hash
> > value of the same type.  So in this case, the caller doesn't need to handle
> > disambiguating raw hashes.
> > 
> > However, asymmetric_verify() also supports ecdsa and ecrdsa signatures.  As far
> > as I can tell, those do *not* have the hash algorithm identifier built-in to the
> > data which is signed/verified; they just sign/verify the data given.  That
> 
> 
> The signatures are generated by evmctl by this code here, which works for
> RSA and ECDSA and likely also ECRDSA:
> 
> https://sourceforge.net/p/linux-ima/ima-evm-utils/ci/master/tree/src/libimaevm.c#l1036
> 
>    if (!EVP_PKEY_sign_init(ctx))
>         goto err;
>     st = "EVP_get_digestbyname";
>     if (!(md = EVP_get_digestbyname(algo)))
>         goto err;
>     st = "EVP_PKEY_CTX_set_signature_md";
>     if (!EVP_PKEY_CTX_set_signature_md(ctx, md))
>         goto err;
>     st = "EVP_PKEY_sign";
>     sigsize = MAX_SIGNATURE_SIZE - sizeof(struct signature_v2_hdr) - 1;
>     if (!EVP_PKEY_sign(ctx, hdr->sig, &sigsize, hash, size))
>         goto err;
>     len = (int)sigsize;
> 
> As far as I know, these are not raw signatures but generate the OIDs for RSA
> + shaXYZ or ECDSA + shaXYZ (1.2.840.10045.4.1 et al) and prepend them to the
> hash and then sign that.

As I said, this appears to be true for RSA but not for ECDSA or ECRDSA.

- Eric

[PATCH v2 6/6] fsverity: update the documentation

[Mimi Zohar]

Update the fsverity documentation related to IMA signature support.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/filesystems/fsverity.rst | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 1d831e3cbcb3..7d8a574a0d3b 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -74,8 +74,12 @@ authenticating the files is up to userspace.  However, to meet some
 users' needs, fs-verity optionally supports a simple signature
 verification mechanism where users can configure the kernel to require
 that all fs-verity files be signed by a key loaded into a keyring; see
-`Built-in signature verification`_.  Support for fs-verity file hashes
-in IMA (Integrity Measurement Architecture) policies is also planned.
+`Built-in signature verification`_.
+
+IMA supports including fs-verity file digests and signatures based
+on the fs-verity file digests in the IMA (Integrity Measurement
+Architecture) measurement list and verifying fs-verity based file
+signatures stored as security.ima xattrs, based on policy.
 
 User API
 ========
@@ -653,13 +657,13 @@ weren't already directly answered in other parts of this document.
     hashed and what to do with those hashes, such as log them,
     authenticate them, or add them to a measurement list.
 
-    IMA is planned to support the fs-verity hashing mechanism as an
-    alternative to doing full file hashes, for people who want the
-    performance and security benefits of the Merkle tree based hash.
-    But it doesn't make sense to force all uses of fs-verity to be
-    through IMA.  As a standalone filesystem feature, fs-verity
-    already meets many users' needs, and it's testable like other
-    filesystem features e.g. with xfstests.
+    IMA supports the fs-verity hashing mechanism as an alternative
+    to doing full file hashes, for people who want the performance
+    and security benefits of the Merkle tree based hash.  But it
+    doesn't make sense to force all uses of fs-verity to be through
+    IMA.  As a standalone filesystem feature, fs-verity already meets
+    many users' needs, and it's testable like other filesystem
+    features e.g. with xfstests.
 
 :Q: Isn't fs-verity useless because the attacker can just modify the
     hashes in the Merkle tree, which is stored on-disk?
-- 
2.27.0