[PATCH v6 0/5] ima: support fs-verity digests and signatures

[PATCH v6 0/5] 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 was discussed prior to fs-verity being upstreamed[1,2].

Including fs-verity file digests and signatures in the IMA measurement
list need to be based on policy and be identifiable.  To address being
based on policy, a new policy rule option "digest_type=verity", applicable
to both "measure" and "appraise" policy rules, is defined.  To address
being identifiable, a new template field 'd-ngv2' and two new template
formats 'ima-ngv2' and 'ima-sigv2' are defined.

d-ngv2:  prefixes the digest type ("ima", "verity") to the digest
algorithm and digest.

ima-ngv2', ima-sigv2: templates with the new d-ngv2 field defined.

In addition the signatures stored in 'security.ima' xattr need to be
disambiguated.  So instead of directly signing the fs-verity digest, the
fs-verity digest is indirectly signed, by signing the hash of the new
ima_file_id structure data (signature version 3) containing the fs-verity
digest and other metadata.

New policy rule option:
appraise_type=sigv3: support for new IMA signature version 3


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

Changelog v6:
- As suggested by Eric Bigger's, instead of defining a new field to
  differentiate between IMA and fs-verity signatures, prepend the
  digest type to the digest field.
- Addressed Eric Bigger's comments: updated the patch description,
  corrected comment, squashed patches, fixed enumeration usage,and
  added assumption to fsverity_get_digest.
- Removed the now unnecessary IMA_VERITY_DIGEST flag
- Updated kernel-parameters.txt

Changelog v5:
- Define ima_max_digest_size struct, removing the locally defined versions.
- Don't overload the 'digest_type=verity' to imply a verity signature,
  but extend the 'appraise_type' policy rule option to define 'sigv3'.

Changelog v4:
- Based on Eric Bigger's signature verification concerns of replacing the
  contents of a file with the ima_file_id struct hash, require per policy
  rule signature versions.
- Addressed Eric Bigger's other comments.
- Added new audit messages "causes".
- Updated patch descriptions.

Changelog v3:
- Addressed Eric Bigger's comments: included Ack, incremented the
  signature format version, the crypto issues are generic and will be
  addressed by him separately.
- Addressed Vitaly Chikunov's comments: hard coded maximum digest size
  rather than using a flexible array, removed unnecessary assignment, and
  fixed comment to match variable name.
- Defined new "ima_max_digest_size" struct to avoid wrapping the
  "ima_digest_data" struct inside a function local structure or
  having to dynamically allocate it with enough memory for the specific
  hash algo size.

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 (5):
  fs-verity: define a function to return the integrity protected file
    digest
  ima: define a new template field named 'd-ngv2' and templates
  ima: permit fsverity's file digests in the IMA measurement list
  ima: support fs-verity file digest based version 3 signatures
  fsverity: update the documentation

 Documentation/ABI/testing/ima_policy          |  36 +++++-
 .../admin-guide/kernel-parameters.txt         |   3 +-
 Documentation/filesystems/fsverity.rst        |  22 ++--
 Documentation/security/IMA-templates.rst      |  12 +-
 fs/verity/Kconfig                             |   1 +
 fs/verity/fsverity_private.h                  |   7 --
 fs/verity/measure.c                           |  43 +++++++
 include/linux/fsverity.h                      |  18 +++
 security/integrity/digsig.c                   |   3 +-
 security/integrity/ima/ima_api.c              |  38 +++++-
 security/integrity/ima/ima_appraise.c         | 114 +++++++++++++++++-
 security/integrity/ima/ima_policy.c           |  68 ++++++++++-
 security/integrity/ima/ima_template.c         |   4 +
 security/integrity/ima/ima_template_lib.c     |  77 ++++++++++--
 security/integrity/ima/ima_template_lib.h     |   4 +
 security/integrity/integrity.h                |  26 +++-
 16 files changed, 431 insertions(+), 45 deletions(-)

-- 
2.27.0

[PATCH v6 1/5] 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).

This assumes that before calling fsverity_get_digest() the file must have
been opened, which is true for the IMA measure/appraise on file open policy
rule use case (func=FILE_CHECK).  do_open() calls vfs_open() immediately
prior to ima_file_check().

Acked-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 fs/verity/Kconfig            |  1 +
 fs/verity/fsverity_private.h |  7 ------
 fs/verity/measure.c          | 43 ++++++++++++++++++++++++++++++++++++
 include/linux/fsverity.h     | 18 +++++++++++++++
 4 files changed, 62 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..e99c00350c28 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -57,3 +57,46 @@ 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.
+ * Assumption: before calling fsverity_get_digest(), the file must have been
+ * opened.
+ *
+ * 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

[PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates

[Mimi Zohar]

In preparation to differentiate between unsigned regular IMA file
hashes and fs-verity's file digests in the IMA measurement list,
define a new template field named 'd-ngv2'.

Also define two new templates named 'ima-ngv2' and 'ima-sigv2', which
include the new 'd-ngv2' field.

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 .../admin-guide/kernel-parameters.txt         |  3 +-
 security/integrity/ima/ima_template.c         |  4 ++
 security/integrity/ima/ima_template_lib.c     | 71 ++++++++++++++++---
 security/integrity/ima/ima_template_lib.h     |  4 ++
 4 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f5a27f067db9..47386ac10471 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1876,7 +1876,8 @@
 
 	ima_template=	[IMA]
 			Select one of defined IMA measurements template formats.
-			Formats: { "ima" | "ima-ng" | "ima-sig" }
+			Formats: { "ima" | "ima-ng" | "ima-ngv2" | "ima-sig" |
+				   "ima-sigv2" }
 			Default: "ima-ng"
 
 	ima_template_fmt=
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index db1ad6d7a57f..c25079faa208 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -20,6 +20,8 @@ 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-sig", .fmt = "d-ng|n-ng|sig"},
+	{.name = "ima-ngv2", .fmt = "d-ngv2|n-ng"},
+	{.name = "ima-sigv2", .fmt = "d-ngv2|n-ng|sig"},
 	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
 	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
 	{.name = "evm-sig",
@@ -38,6 +40,8 @@ static const struct ima_template_field supported_fields[] = {
 	 .field_show = ima_show_template_string},
 	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
 	 .field_show = ima_show_template_digest_ng},
+	{.field_id = "d-ngv2", .field_init = ima_eventdigest_ngv2_init,
+	 .field_show = ima_show_template_digest_ngv2},
 	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
 	 .field_show = ima_show_template_string},
 	{.field_id = "sig", .field_init = ima_eventsig_init,
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 7155d17a3b75..bd95864a5f6f 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -24,11 +24,16 @@ static bool ima_template_hash_algo_allowed(u8 algo)
 enum data_formats {
 	DATA_FMT_DIGEST = 0,
 	DATA_FMT_DIGEST_WITH_ALGO,
+	DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
 	DATA_FMT_STRING,
 	DATA_FMT_HEX,
 	DATA_FMT_UINT
 };
 
+#define DIGEST_TYPE_MAXLEN 16	/* including NULL */
+static const char * const digest_type_name[] = {"ima"};
+static int digest_type_size = ARRAY_SIZE(digest_type_name);
+
 static int ima_write_template_field_data(const void *data, const u32 datalen,
 					 enum data_formats datafmt,
 					 struct ima_field_data *field_data)
@@ -72,8 +77,9 @@ static void ima_show_template_data_ascii(struct seq_file *m,
 	u32 buflen = field_data->len;
 
 	switch (datafmt) {
+	case DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
 	case DATA_FMT_DIGEST_WITH_ALGO:
-		buf_ptr = strnchr(field_data->data, buflen, ':');
+		buf_ptr = strrchr(field_data->data, ':');
 		if (buf_ptr != field_data->data)
 			seq_printf(m, "%s", field_data->data);
 
@@ -178,6 +184,14 @@ void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
 				     field_data);
 }
 
+void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
+				   struct ima_field_data *field_data)
+{
+	ima_show_template_field_data(m, show,
+				     DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
+				     field_data);
+}
+
 void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data)
 {
@@ -265,26 +279,39 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
 }
 
 static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
-				       u8 hash_algo,
+				       u8 digest_type, u8 hash_algo,
 				       struct ima_field_data *field_data)
 {
 	/*
 	 * digest formats:
 	 *  - DATA_FMT_DIGEST: digest
 	 *  - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
+	 *  - DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
+	 *	[<digest type> + ':' + <hash algo>] + ':' + '\0' + digest,
+	 *    where <hash type> is either "ima" or "verity",
 	 *    where <hash algo> is provided if the hash algorithm is not
 	 *    SHA1 or MD5
 	 */
-	u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
+	u8 buffer[DIGEST_TYPE_MAXLEN + CRYPTO_MAX_ALG_NAME + 2 +
+		IMA_MAX_DIGEST_SIZE] = { 0 };
 	enum data_formats fmt = DATA_FMT_DIGEST;
 	u32 offset = 0;
 
-	if (hash_algo < HASH_ALGO__LAST) {
-		fmt = DATA_FMT_DIGEST_WITH_ALGO;
-		offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1, "%s",
+	if (digest_type < digest_type_size && hash_algo < HASH_ALGO__LAST) {
+		fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
+		offset += snprintf(buffer, DIGEST_TYPE_MAXLEN +
+				   CRYPTO_MAX_ALG_NAME + 1, "%*s:%s",
+				   (int)strlen(digest_type_name[digest_type]),
+				   digest_type_name[digest_type],
 				   hash_algo_name[hash_algo]);
 		buffer[offset] = ':';
 		offset += 2;
+	} else if (hash_algo < HASH_ALGO__LAST) {
+		fmt = DATA_FMT_DIGEST_WITH_ALGO;
+		offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1,
+				   "%s", hash_algo_name[hash_algo]);
+		buffer[offset] = ':';
+		offset += 2;
 	}
 
 	if (digest)
@@ -359,7 +386,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
 	cur_digestsize = hash.hdr.length;
 out:
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
-					   HASH_ALGO__LAST, field_data);
+					   digest_type_size, HASH_ALGO__LAST,
+					   field_data);
 }
 
 /*
@@ -380,7 +408,31 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
 	hash_algo = event_data->iint->ima_hash->algo;
 out:
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
-					   hash_algo, field_data);
+					   digest_type_size, hash_algo,
+					   field_data);
+}
+
+/*
+ * This function writes the digest of an event (without size limit),
+ * prefixed with both the hash type and algorithm.
+ */
+int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
+			      struct ima_field_data *field_data)
+{
+	u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
+	u32 cur_digestsize = 0;
+	u8 digest_type = 0;
+
+	if (event_data->violation)	/* recording a violation. */
+		goto out;
+
+	cur_digest = event_data->iint->ima_hash->digest;
+	cur_digestsize = event_data->iint->ima_hash->length;
+
+	hash_algo = event_data->iint->ima_hash->algo;
+out:
+	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
+					   digest_type, hash_algo, field_data);
 }
 
 /*
@@ -415,7 +467,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
 	}
 
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
-					   hash_algo, field_data);
+					   digest_type_size, hash_algo,
+					   field_data);
 }
 
 static int ima_eventname_init_common(struct ima_event_data *event_data,
diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
index c71f1de95753..9f7c335f304f 100644
--- a/security/integrity/ima/ima_template_lib.h
+++ b/security/integrity/ima/ima_template_lib.h
@@ -21,6 +21,8 @@ void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
 				 struct ima_field_data *field_data);
+void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
+				   struct ima_field_data *field_data);
 void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
 			      struct ima_field_data *field_data);
 void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
@@ -38,6 +40,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_ngv2_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 v6 3/5] ima: permit fsverity's file digests in the IMA measurement list

[Mimi Zohar]

Permit fsverity's file digest (a hash of struct fsverity_digest) to be
included in the IMA measurement list, based on the new measurement
policy rule 'digest_type=verity' option.

To differentiate between a regular IMA file hash from an fsverity's
file digest, use the new d-ngv2 format field included in the ima-ngv2
template.

The following policy rule requires fsverity file digests and specifies
the new 'ima-ngv2' template, which contains the new 'd-ngv2' field.  The
policy rule may be constrained, for example based on a fsuuid or LSM
label.

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

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy      |  9 ++++++
 Documentation/security/IMA-templates.rst  |  8 +++++
 security/integrity/ima/ima_api.c          | 38 +++++++++++++++++++++--
 security/integrity/ima/ima_policy.c       | 38 ++++++++++++++++++++++-
 security/integrity/ima/ima_template_lib.c |  4 ++-
 security/integrity/integrity.h            |  3 +-
 6 files changed, 95 insertions(+), 5 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 839fab811b18..2e0c501ce9c8 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -51,6 +51,9 @@ Description:
 			appraise_flag:= [check_blacklist]
 			Currently, blacklist check is only for files signed with appended
 			signature.
+			digest_type:= verity
+			    Require fs-verity's file digest instead of the
+			    regular IMA file hash.
 			keyrings:= list of keyrings
 			(eg, .builtin_trusted_keys|.ima). Only valid
 			when action is "measure" and func is KEY_CHECK.
@@ -149,3 +152,9 @@ Description:
 		security.ima xattr of a file:
 
 			appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
+
+		Example of a 'measure' rule requiring fs-verity's digests
+		with indication of type of digest in the measurement list.
+
+			measure func=FILE_CHECK digest_type=verity \
+				template=ima-ngv2
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 1a91d92950a7..2d4789dc7750 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,6 +68,9 @@ descriptors by adding their identifier to the format string
  - 'd-ng': the digest of the event, calculated with an arbitrary hash
    algorithm (field format: [<hash algo>:]digest, where the digest
    prefix is shown only if the hash algorithm is not SHA1 or MD5);
+ - 'd-ngv2': same as d-ng, but prefixed with the digest type.
+    field format: [<digest type>:<hash algo>:]digest,
+        where the digest type is either "ima" or "verity".
  - 'd-modsig': the digest of the event without the appended modsig;
  - 'n-ng': the name of the event, without size limitations;
  - 'sig': the file signature, or the EVM portable signature if the file
@@ -106,3 +109,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 c6805af46211..525b13916745 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_max_digest_data *hash)
+{
+	u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
+	enum hash_algo verity_alg;
+	int ret;
+
+	ret = fsverity_get_digest(iint->inode, verity_digest, &verity_alg);
+	if (ret)
+		return ret;
+	if (hash->hdr.algo != verity_alg)
+		return -EINVAL;
+	hash->hdr.length = hash_digest_size[verity_alg];
+	memcpy(hash->hdr.digest, verity_digest, hash->hdr.length);
+	return 0;
+}
+
 /*
  * ima_collect_measurement - collect file measurement
  *
@@ -242,14 +260,30 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	 */
 	i_version = inode_query_iversion(inode);
 	hash.hdr.algo = algo;
+	hash.hdr.length = hash_digest_size[algo];
 
 	/* 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_REQUIRED) {
+		result = ima_get_verity_digest(iint, &hash);
+		switch (result) {
+		case 0:
+			break;
+		case -ENODATA:
+			audit_cause = "no-verity-digest";
+			result = -EINVAL;
+			break;
+		case -EINVAL:
+		default:
+			audit_cause = "invalid-verity-digest";
+			break;
+		}
+	} 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 a0f3775cbd82..c6b0454b2e25 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_REQUIRED))
 			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, "verity")) == 0)
+				entry->flags |= IMA_VERITY_REQUIRED;
+			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-ngv2 template field recommended for unsigned fs-verity digests */
+	if (!result && entry->action == MEASURE &&
+	    entry->flags & IMA_VERITY_REQUIRED) {
+		template_desc = entry->template ? entry->template :
+						  ima_template_desc_current();
+		check_template_field(template_desc, "d-ngv2",
+				     "verity rules should include d-ngv2");
+	}
+
 	audit_log_format(ab, "res=%d", !result);
 	audit_log_end(ab);
 	return result;
@@ -2155,6 +2189,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 		else
 			seq_puts(m, "appraise_type=imasig ");
 	}
+	if (entry->flags & IMA_VERITY_REQUIRED)
+		seq_puts(m, "digest_type=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 bd95864a5f6f..0cff6658a4c2 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -31,7 +31,7 @@ enum data_formats {
 };
 
 #define DIGEST_TYPE_MAXLEN 16	/* including NULL */
-static const char * const digest_type_name[] = {"ima"};
+static const char * const digest_type_name[] = {"ima", "verity"};
 static int digest_type_size = ARRAY_SIZE(digest_type_name);
 
 static int ima_write_template_field_data(const void *data, const u32 datalen,
@@ -430,6 +430,8 @@ int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
 	cur_digestsize = event_data->iint->ima_hash->length;
 
 	hash_algo = event_data->iint->ima_hash->algo;
+	if (event_data->iint->flags & IMA_VERITY_REQUIRED)
+		digest_type = 1;
 out:
 	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
 					   digest_type, hash_algo, field_data);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index daf49894fd7d..d42a01903f08 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -32,7 +32,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
@@ -40,6 +40,7 @@
 #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
 #define IMA_MODSIG_ALLOWED	0x20000000
 #define IMA_CHECK_BLACKLIST	0x40000000
+#define IMA_VERITY_REQUIRED	0x80000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.27.0

[PATCH v6 4/5] ima: support fs-verity file digest based version 3 signatures

[Mimi Zohar]

IMA may verify a file's integrity against a "good" value stored in the
'security.ima' xattr or as an appended signature, based on policy.  When
the "good value" is stored in the xattr, the xattr may contain a file
hash or signature.  In either case, the "good" value is preceded by a
header.  The first byte of the xattr header indicates the type of data
- hash, signature - stored in the xattr.  To support storing fs-verity
signatures in the 'security.ima' xattr requires further differentiating
the fs-verity signature from the existing IMA signature.

In addition the signatures stored in 'security.ima' xattr, need to be
disambiguated.  Instead of directly signing the fs-verity digest, a new
signature version 3 is defined as the hash of the ima_file_id structure,
which identifies the type of signature and the digest.

The IMA policy defines "which" files are to be measured, verified, and/or
audited.  For those files being verified, the policy rules indicate "how"
the file should be verified.  For example to require a file be signed,
the appraise policy rule must include the 'appraise_type' option.

	appraise_type:= [imasig] | [imasig|modsig] | [sigv3]
           where 'imasig' is the original or v2 signature (default),
           where 'modsig' is an appended signature,
           where 'sigv3' is the signature v3 format.

The policy rule must also indicate the type of signature, if not the IMA
default, by specifying the digest type:

	digest_type:= [verity]

The following policy rule requires fsverity signatures.  The rule may be
constrained, for example, based on a fsuuid or LSM label.

      appraise func=BPRM_CHECK digest_type=verity appraise_type=sigv3

Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
---
 Documentation/ABI/testing/ima_policy      |  27 ++++-
 Documentation/security/IMA-templates.rst  |   4 +-
 security/integrity/digsig.c               |   3 +-
 security/integrity/ima/ima_appraise.c     | 114 +++++++++++++++++++++-
 security/integrity/ima/ima_policy.c       |  30 +++++-
 security/integrity/ima/ima_template_lib.c |   4 +-
 security/integrity/integrity.h            |  23 ++++-
 7 files changed, 190 insertions(+), 15 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 2e0c501ce9c8..acd17183ad8c 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -47,7 +47,11 @@ Description:
 			fgroup:= decimal value
 		  lsm:  are LSM specific
 		  option:
-			appraise_type:= [imasig] [imasig|modsig]
+			appraise_type:= [imasig] | [imasig|modsig] | [sigv3]
+			    where 'imasig' is the original or v2 signature,
+			    where 'modsig' is an appended signature,
+			    where 'sigv3' is the IMA v3 signature.
+
 			appraise_flag:= [check_blacklist]
 			Currently, blacklist check is only for files signed with appended
 			signature.
@@ -158,3 +162,24 @@ Description:
 
 			measure func=FILE_CHECK digest_type=verity \
 				template=ima-ngv2
+
+		Example of 'measure' and 'appraise' rules requiring fs-verity
+		signatures (version 3) stored in security.ima xattr.
+
+		The 'measure' rule specifies the 'ima-sigv2' template option,
+		which includes the indication of type of digest and the file
+		signature in the measurement list.
+
+			measure func=BPRM_CHECK digest_type=verity \
+				template=ima-sigv2
+
+
+		The 'appraise' rule specifies the type and signature version
+		(sigv3) required.
+
+			appraise func=BPRM_CHECK digest_type=verity \
+				appraise_type=sigv3
+
+		All of these policy rules could, for example, be constrained
+		either based on a filesystem's UUID (fsuuid) or based on LSM
+		labels.
diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
index 2d4789dc7750..06cddcd510da 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -73,8 +73,8 @@ descriptors by adding their identifier to the format string
         where the digest type is either "ima" or "verity".
  - 'd-modsig': the digest of the event without the appended modsig;
  - '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 'security.ima' contains a file hash.
  - '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/security/integrity/digsig.c b/security/integrity/digsig.c
index 3b06a01bd0fd..56472bc8e44c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -74,7 +74,8 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 		/* v1 API expect signature without xattr type */
 		return digsig_verify(keyring, sig + 1, siglen - 1, digest,
 				     digestlen);
-	case 2:
+	case 2: /* regular file data hash based signature */
+	case 3: /* struct ima_file_id data base signature */
 		return asymmetric_verify(keyring, sig, siglen, digest,
 					 digestlen);
 	}
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 17232bbfb9f9..e76d88b309a2 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -13,7 +13,9 @@
 #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 "ima.h"
 
@@ -183,13 +185,18 @@ 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:
+		sig = (typeof(sig))xattr_value;
+		if (sig->version != 3 || xattr_len <= sizeof(*sig) ||
+		    sig->hash_algo >= HASH_ALGO__LAST)
+			return ima_hash_algo;
+		return sig->hash_algo;
 	case EVM_IMA_XATTR_DIGSIG:
 		sig = (typeof(sig))xattr_value;
 		if (sig->version != 2 || xattr_len <= sizeof(*sig)
 		    || sig->hash_algo >= HASH_ALGO__LAST)
 			return ima_hash_algo;
 		return sig->hash_algo;
-		break;
 	case IMA_XATTR_DIGEST_NG:
 		/* first byte contains algorithm id */
 		ret = xattr_value->data[0];
@@ -225,6 +232,40 @@ int ima_read_xattr(struct dentry *dentry,
 	return ret;
 }
 
+/*
+ * calc_file_id_hash - calculate the hash of the ima_file_id struct data
+ * @type: xattr type [enum evm_ima_xattr_type]
+ * @algo: hash algorithm [enum hash_algo]
+ * @digest: pointer to the digest to be hashed
+ * @hash: (out) pointer to the hash
+ *
+ * IMA signature version 3 disambiguates the data that is signed by
+ * indirectly signing the hash of the ima_file_id structure data.
+ *
+ * Signing the ima_file_id struct is currently only supported for
+ * IMA_VERITY_DIGSIG type xattrs.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int calc_file_id_hash(enum evm_ima_xattr_type type,
+			     enum hash_algo algo, const u8 *digest,
+			     struct ima_digest_data *hash)
+{
+	struct ima_file_id file_id = {
+		.hash_type = IMA_VERITY_DIGSIG, .hash_algorithm = algo};
+	uint unused = HASH_MAX_DIGESTSIZE - hash_digest_size[algo];
+
+	if (type != IMA_VERITY_DIGSIG)
+		return -EINVAL;
+
+	memcpy(file_id.hash, digest, hash_digest_size[algo]);
+
+	hash->algo = algo;
+	hash->length = hash_digest_size[algo];
+
+	return ima_calc_buffer_hash(&file_id, sizeof(file_id) - unused, hash);
+}
+
 /*
  * xattr_verify - verify xattr digest or signature
  *
@@ -236,7 +277,10 @@ 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_max_digest_data hash;
+	struct signature_v2_hdr *sig;
 	int rc = -EINVAL, hash_start = 0;
+	int mask;
 
 	switch (xattr_value->type) {
 	case IMA_XATTR_DIGEST_NG:
@@ -246,7 +290,10 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 	case IMA_XATTR_DIGEST:
 		if (*status != INTEGRITY_PASS_IMMUTABLE) {
 			if (iint->flags & IMA_DIGSIG_REQUIRED) {
-				*cause = "IMA-signature-required";
+				if (iint->flags & IMA_VERITY_REQUIRED)
+					*cause = "verity-signature-required";
+				else
+					*cause = "IMA-signature-required";
 				*status = INTEGRITY_FAIL;
 				break;
 			}
@@ -274,6 +321,20 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 		break;
 	case EVM_IMA_XATTR_DIGSIG:
 		set_bit(IMA_DIGSIG, &iint->atomic_flags);
+
+		mask = IMA_DIGSIG_REQUIRED | IMA_VERITY_REQUIRED;
+		if ((iint->flags & mask) == mask) {
+			*cause = "verity-signature-required";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+
+		sig = (typeof(sig))xattr_value;
+		if (sig->version != 2) {
+			*cause = "invalid-signature-version";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
 		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
 					     (const char *)xattr_value,
 					     xattr_len,
@@ -296,6 +357,44 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
 		} else {
 			*status = INTEGRITY_PASS;
 		}
+		break;
+	case IMA_VERITY_DIGSIG:
+		set_bit(IMA_DIGSIG, &iint->atomic_flags);
+
+		if (iint->flags & IMA_DIGSIG_REQUIRED) {
+			if (!(iint->flags & IMA_VERITY_REQUIRED)) {
+				*cause = "IMA-signature-required";
+				*status = INTEGRITY_FAIL;
+				break;
+			}
+		}
+
+		sig = (typeof(sig))xattr_value;
+		if (sig->version != 3) {
+			*cause = "invalid-signature-version";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+
+		rc = calc_file_id_hash(IMA_VERITY_DIGSIG, iint->ima_hash->algo,
+				       iint->ima_hash->digest, &hash.hdr);
+		if (rc) {
+			*cause = "sigv3-hashing-error";
+			*status = INTEGRITY_FAIL;
+			break;
+		}
+
+		rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+					     (const char *)xattr_value,
+					     xattr_len, hash.digest,
+					     hash.hdr.length);
+		if (rc) {
+			*cause = "invalid-verity-signature";
+			*status = INTEGRITY_FAIL;
+		} else {
+			*status = INTEGRITY_PASS;
+		}
+
 		break;
 	default:
 		*status = INTEGRITY_UNKNOWN;
@@ -396,8 +495,15 @@ int ima_appraise_measurement(enum ima_hooks func,
 		if (rc && rc != -ENODATA)
 			goto out;
 
-		cause = iint->flags & IMA_DIGSIG_REQUIRED ?
-				"IMA-signature-required" : "missing-hash";
+		if (iint->flags & IMA_DIGSIG_REQUIRED) {
+			if (iint->flags & IMA_VERITY_REQUIRED)
+				cause = "verity-signature-required";
+			else
+				cause = "IMA-signature-required";
+		} else {
+			cause = "missing-hash";
+		}
+
 		status = INTEGRITY_NOLABEL;
 		if (file->f_mode & FMODE_CREATED)
 			iint->flags |= IMA_NEW_FILE;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c6b0454b2e25..fbdea5c0cdc6 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1311,6 +1311,16 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 	    !(entry->flags & IMA_MODSIG_ALLOWED))
 		return false;
 
+	/*
+	 * To avoid 'appraise_type=sigv3' and 'digest_type=verity' ordering
+	 * requirements, ensure both have been specified for file verity
+	 * signatures.
+	 */
+	if (entry->action == APPRAISE &&
+	    (entry->flags & IMA_VERITY_REQUIRED) &&
+	    !(entry->flags & IMA_DIGSIG_REQUIRED))
+		return false;
+
 	return true;
 }
 
@@ -1735,14 +1745,24 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			break;
 		case Opt_appraise_type:
 			ima_log_string(ab, "appraise_type", args[0].from);
-			if ((strcmp(args[0].from, "imasig")) == 0)
+			if ((strcmp(args[0].from, "imasig")) == 0) {
 				entry->flags |= IMA_DIGSIG_REQUIRED;
-			else if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) &&
-				 strcmp(args[0].from, "imasig|modsig") == 0)
+			} else if (strcmp(args[0].from, "sigv3") == 0) {
+				/*
+				 * Only fsverity supports sigv3 for now.
+				 * No need to define a new flag.
+				 */
+				if (entry->flags & IMA_VERITY_REQUIRED)
+					entry->flags |= IMA_DIGSIG_REQUIRED;
+				else
+					result = -EINVAL;
+			} else if (IS_ENABLED(CONFIG_IMA_APPRAISE_MODSIG) &&
+				 strcmp(args[0].from, "imasig|modsig") == 0) {
 				entry->flags |= IMA_DIGSIG_REQUIRED |
 						IMA_MODSIG_ALLOWED;
-			else
+			} else {
 				result = -EINVAL;
+			}
 			break;
 		case Opt_appraise_flag:
 			ima_log_string(ab, "appraise_flag", args[0].from);
@@ -2186,6 +2206,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 	if (entry->flags & IMA_DIGSIG_REQUIRED) {
 		if (entry->flags & IMA_MODSIG_ALLOWED)
 			seq_puts(m, "appraise_type=imasig|modsig ");
+		else if (entry->flags & IMA_VERITY_REQUIRED)
+			seq_puts(m, "appraise_type=sigv3 ");
 		else
 			seq_puts(m, "appraise_type=imasig ");
 	}
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index 0cff6658a4c2..80fd2d908315 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -530,7 +530,9 @@ 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 &&
+	     xattr_value->type != 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 d42a01903f08..44418f0ec0ab 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
 };
 
@@ -93,7 +94,7 @@ struct evm_xattr {
 	u8 digest[SHA1_DIGEST_SIZE];
 } __packed;
 
-#define IMA_MAX_DIGEST_SIZE	64
+#define IMA_MAX_DIGEST_SIZE	HASH_MAX_DIGESTSIZE
 
 struct ima_digest_data {
 	u8 algo;
@@ -122,7 +123,11 @@ struct ima_max_digest_data {
 } __packed;
 
 /*
- * signature format v2 - for using with asymmetric keys
+ * signature header format v2 - for using with asymmetric keys
+ *
+ * signature format:
+ * version 2: regular file data hash based signature
+ * version 3: struct ima_file_id data based signature
  */
 struct signature_v2_hdr {
 	uint8_t type;		/* xattr type */
@@ -133,6 +138,20 @@ struct signature_v2_hdr {
 	uint8_t sig[];		/* signature payload */
 } __packed;
 
+/*
+ * IMA signature version 3 disambiguates the data that is signed, by
+ * indirectly signing the hash of the ima_file_id structure data,
+ * containing either the fsverity_descriptor struct digest or, in the
+ * future, the regular IMA file hash.
+ *
+ * (The hash of the ima_file_id structure is only of the portion used.)
+ */
+struct ima_file_id {
+	__u8 hash_type;		/* xattr type [enum evm_ima_xattr_type] */
+	__u8 hash_algorithm;	/* Digest algorithm [enum hash_algo] */
+	__u8 hash[HASH_MAX_DIGESTSIZE];
+} __packed;
+
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
-- 
2.27.0

[PATCH v6 5/5] 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..28a47488848e 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 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

Re: [PATCH v6 5/5] fsverity: update the documentation

[Stefan Berger]

On 3/18/22 14:21, Mimi Zohar wrote:
> 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..28a47488848e 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 in the
> +IMA (Integrity Measurement Architecture) measurement list and

The Integrity Measurement Architecture (IMA) supports including ...

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

IMA supports the fs-verity hashing mechanism as an alternative to full 
file hashes for those who want the performance and security benefits ...

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

However, it doesn't make sense ...

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

Fs-verity already meets many user' needs even as a standalone filesystem 
feature and it is testable like other ...

>   
>   :Q: Isn't fs-verity useless because the attacker can just modify the
>       hashes in the Merkle tree, which is stored on-disk?

Re: [PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates

[Stefan Berger]

On 3/18/22 14:21, Mimi Zohar wrote:
> In preparation to differentiate between unsigned regular IMA file
> hashes and fs-verity's file digests in the IMA measurement list,
> define a new template field named 'd-ngv2'.
> 
> Also define two new templates named 'ima-ngv2' and 'ima-sigv2', which
> include the new 'd-ngv2' field.
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   .../admin-guide/kernel-parameters.txt         |  3 +-
>   security/integrity/ima/ima_template.c         |  4 ++
>   security/integrity/ima/ima_template_lib.c     | 71 ++++++++++++++++---
>   security/integrity/ima/ima_template_lib.h     |  4 ++
>   4 files changed, 72 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index f5a27f067db9..47386ac10471 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1876,7 +1876,8 @@
>   
>   	ima_template=	[IMA]
>   			Select one of defined IMA measurements template formats.
> -			Formats: { "ima" | "ima-ng" | "ima-sig" }
> +			Formats: { "ima" | "ima-ng" | "ima-ngv2" | "ima-sig" |
> +				   "ima-sigv2" }
>   			Default: "ima-ng"
>   
>   	ima_template_fmt=
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index db1ad6d7a57f..c25079faa208 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -20,6 +20,8 @@ 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-sig", .fmt = "d-ng|n-ng|sig"},
> +	{.name = "ima-ngv2", .fmt = "d-ngv2|n-ng"},
> +	{.name = "ima-sigv2", .fmt = "d-ngv2|n-ng|sig"},
>   	{.name = "ima-buf", .fmt = "d-ng|n-ng|buf"},
>   	{.name = "ima-modsig", .fmt = "d-ng|n-ng|sig|d-modsig|modsig"},
>   	{.name = "evm-sig",
> @@ -38,6 +40,8 @@ static const struct ima_template_field supported_fields[] = {
>   	 .field_show = ima_show_template_string},
>   	{.field_id = "d-ng", .field_init = ima_eventdigest_ng_init,
>   	 .field_show = ima_show_template_digest_ng},
> +	{.field_id = "d-ngv2", .field_init = ima_eventdigest_ngv2_init,
> +	 .field_show = ima_show_template_digest_ngv2},
>   	{.field_id = "n-ng", .field_init = ima_eventname_ng_init,
>   	 .field_show = ima_show_template_string},
>   	{.field_id = "sig", .field_init = ima_eventsig_init,
> diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
> index 7155d17a3b75..bd95864a5f6f 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -24,11 +24,16 @@ static bool ima_template_hash_algo_allowed(u8 algo)
>   enum data_formats {
>   	DATA_FMT_DIGEST = 0,
>   	DATA_FMT_DIGEST_WITH_ALGO,
> +	DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
>   	DATA_FMT_STRING,
>   	DATA_FMT_HEX,
>   	DATA_FMT_UINT
>   };
>   
> +#define DIGEST_TYPE_MAXLEN 16	/* including NULL */
> +static const char * const digest_type_name[] = {"ima"};
> +static int digest_type_size = ARRAY_SIZE(digest_type_name);
> +
>   static int ima_write_template_field_data(const void *data, const u32 datalen,
>   					 enum data_formats datafmt,
>   					 struct ima_field_data *field_data)
> @@ -72,8 +77,9 @@ static void ima_show_template_data_ascii(struct seq_file *m,
>   	u32 buflen = field_data->len;
>   
>   	switch (datafmt) {
> +	case DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
>   	case DATA_FMT_DIGEST_WITH_ALGO:
> -		buf_ptr = strnchr(field_data->data, buflen, ':');
> +		buf_ptr = strrchr(field_data->data, ':');
>   		if (buf_ptr != field_data->data)
>   			seq_printf(m, "%s", field_data->data);
>   
> @@ -178,6 +184,14 @@ void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
>   				     field_data);
>   }
>   
> +void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
> +				   struct ima_field_data *field_data)
> +{
> +	ima_show_template_field_data(m, show,
> +				     DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO,
> +				     field_data);
> +}
> +
>   void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
>   			      struct ima_field_data *field_data)
>   {
> @@ -265,26 +279,39 @@ int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp,
>   }
>   
>   static int ima_eventdigest_init_common(const u8 *digest, u32 digestsize,
> -				       u8 hash_algo,
> +				       u8 digest_type, u8 hash_algo,
>   				       struct ima_field_data *field_data)
>   {
>   	/*
>   	 * digest formats:
>   	 *  - DATA_FMT_DIGEST: digest
>   	 *  - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
> +	 *  - DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
> +	 *	[<digest type> + ':' + <hash algo>] + ':' + '\0' + digest,
> +	 *    where <hash type> is either "ima" or "verity",
>   	 *    where <hash algo> is provided if the hash algorithm is not
>   	 *    SHA1 or MD5
>   	 */
> -	u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> +	u8 buffer[DIGEST_TYPE_MAXLEN + CRYPTO_MAX_ALG_NAME + 2 +
> +		IMA_MAX_DIGEST_SIZE] = { 0 };

Should it not be DIGEST_TYPE_MAXLEN + 1 /* for ':' */ + 
CRYPTO_MAX_ALG_NAME + ....

>   	enum data_formats fmt = DATA_FMT_DIGEST;
>   	u32 offset = 0;
>   
> -	if (hash_algo < HASH_ALGO__LAST) {
> -		fmt = DATA_FMT_DIGEST_WITH_ALGO;
> -		offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1, "%s",
> +	if (digest_type < digest_type_size && hash_algo < HASH_ALGO__LAST) {

It's a bit difficult to see what the digest_type has to do with size...
Maybe digest_type should be a enum and the comparison here should be 
digest_type != DIGEST_TYPE_NONE or something like it..


> +		fmt = DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO;
> +		offset += snprintf(buffer, DIGEST_TYPE_MAXLEN +
> +				   CRYPTO_MAX_ALG_NAME + 1, "%*s:%s",
> +				   (int)strlen(digest_type_name[digest_type]),
> +				   digest_type_name[digest_type],
>   				   hash_algo_name[hash_algo]);
>   		buffer[offset] = ':';
>   		offset += 2;
> +	} else if (hash_algo < HASH_ALGO__LAST) {
> +		fmt = DATA_FMT_DIGEST_WITH_ALGO;
> +		offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1,
> +				   "%s", hash_algo_name[hash_algo]);
> +		buffer[offset] = ':';
> +		offset += 2;
>   	}
>   
>   	if (digest)
> @@ -359,7 +386,8 @@ int ima_eventdigest_init(struct ima_event_data *event_data,
>   	cur_digestsize = hash.hdr.length;
>   out:
>   	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> -					   HASH_ALGO__LAST, field_data);
> +					   digest_type_size, HASH_ALGO__LAST,
> +					   field_data);
>   }
>   
>   /*
> @@ -380,7 +408,31 @@ int ima_eventdigest_ng_init(struct ima_event_data *event_data,
>   	hash_algo = event_data->iint->ima_hash->algo;
>   out:
>   	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> -					   hash_algo, field_data);
> +					   digest_type_size, hash_algo,
> +					   field_data);
> +}
> +
> +/*
> + * This function writes the digest of an event (without size limit),
> + * prefixed with both the hash type and algorithm.
> + */
> +int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
> +			      struct ima_field_data *field_data)
> +{
> +	u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
> +	u32 cur_digestsize = 0;
> +	u8 digest_type = 0;

What does '0' mean? I think this should definitely be an enum or at 
least #define.

> +
> +	if (event_data->violation)	/* recording a violation. */
> +		goto out;
> +
> +	cur_digest = event_data->iint->ima_hash->digest;
> +	cur_digestsize = event_data->iint->ima_hash->length;
> +
> +	hash_algo = event_data->iint->ima_hash->algo;
> +out:
> +	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> +					   digest_type, hash_algo, field_data);
>   }
>   
>   /*
> @@ -415,7 +467,8 @@ int ima_eventdigest_modsig_init(struct ima_event_data *event_data,
>   	}
>   
>   	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
> -					   hash_algo, field_data);
> +					   digest_type_size, hash_algo,
> +					   field_data);
>   }
>   
>   static int ima_eventname_init_common(struct ima_event_data *event_data,
> diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h
> index c71f1de95753..9f7c335f304f 100644
> --- a/security/integrity/ima/ima_template_lib.h
> +++ b/security/integrity/ima/ima_template_lib.h
> @@ -21,6 +21,8 @@ void ima_show_template_digest(struct seq_file *m, enum ima_show_type show,
>   			      struct ima_field_data *field_data);
>   void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show,
>   				 struct ima_field_data *field_data);
> +void ima_show_template_digest_ngv2(struct seq_file *m, enum ima_show_type show,
> +				   struct ima_field_data *field_data);
>   void ima_show_template_string(struct seq_file *m, enum ima_show_type show,
>   			      struct ima_field_data *field_data);
>   void ima_show_template_sig(struct seq_file *m, enum ima_show_type show,
> @@ -38,6 +40,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_ngv2_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,

Re: [PATCH v6 5/5] fsverity: update the documentation

[Mimi Zohar]

On Fri, 2022-03-18 at 16:55 -0400, Stefan Berger wrote:
> 
> On 3/18/22 14:21, Mimi Zohar wrote:
> > 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..28a47488848e 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 in the
> > +IMA (Integrity Measurement Architecture) measurement list and
> 
> The Integrity Measurement Architecture (IMA) supports including ...
> 
> > +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
> 
> IMA supports the fs-verity hashing mechanism as an alternative to full 
> file hashes for those who want the performance and security benefits ...
> 
> > +    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
> 
> However, it doesn't make sense ...
> 
> > +    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.
> 
> Fs-verity already meets many user' needs even as a standalone filesystem 
> feature and it is testable like other ...
> 
> >   
> >   :Q: Isn't fs-verity useless because the attacker can just modify the
> >       hashes in the Merkle tree, which is stored on-disk?

Thanks, Stefan, for the suggestions.  I tried to minimize the changes
as much as possible.  Based on another thread, the documentation should
be updated, but I'm not going to be presumptuous and make those
changes.  Eric, should I drop this patch and let you update the fs-
verity documentation as you want?

-- 
thanks,

Mimi

Re: [PATCH v6 3/5] ima: permit fsverity's file digests in the IMA measurement list

[Stefan Berger]

On 3/18/22 14:21, Mimi Zohar wrote:
> Permit fsverity's file digest (a hash of struct fsverity_digest) to be
> included in the IMA measurement list, based on the new measurement
> policy rule 'digest_type=verity' option.
> 
> To differentiate between a regular IMA file hash from an fsverity's
> file digest, use the new d-ngv2 format field included in the ima-ngv2
> template.
> 
> The following policy rule requires fsverity file digests and specifies
> the new 'ima-ngv2' template, which contains the new 'd-ngv2' field.  The
> policy rule may be constrained, for example based on a fsuuid or LSM
> label.
> 
> measure func=FILE_CHECK digest_type=verity template=ima-ngv2
> 
> Signed-off-by: Mimi Zohar <zohar@linux.ibm.com>
> ---
>   Documentation/ABI/testing/ima_policy      |  9 ++++++
>   Documentation/security/IMA-templates.rst  |  8 +++++
>   security/integrity/ima/ima_api.c          | 38 +++++++++++++++++++++--
>   security/integrity/ima/ima_policy.c       | 38 ++++++++++++++++++++++-
>   security/integrity/ima/ima_template_lib.c |  4 ++-
>   security/integrity/integrity.h            |  3 +-
>   6 files changed, 95 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 839fab811b18..2e0c501ce9c8 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -51,6 +51,9 @@ Description:
>   			appraise_flag:= [check_blacklist]
>   			Currently, blacklist check is only for files signed with appended
>   			signature.
> +			digest_type:= verity
> +			    Require fs-verity's file digest instead of the
> +			    regular IMA file hash.
>   			keyrings:= list of keyrings
>   			(eg, .builtin_trusted_keys|.ima). Only valid
>   			when action is "measure" and func is KEY_CHECK.
> @@ -149,3 +152,9 @@ Description:
>   		security.ima xattr of a file:
>   
>   			appraise func=SETXATTR_CHECK appraise_algos=sha256,sha384,sha512
> +
> +		Example of a 'measure' rule requiring fs-verity's digests
> +		with indication of type of digest in the measurement list.
> +
> +			measure func=FILE_CHECK digest_type=verity \
> +				template=ima-ngv2
> diff --git a/Documentation/security/IMA-templates.rst b/Documentation/security/IMA-templates.rst
> index 1a91d92950a7..2d4789dc7750 100644
> --- a/Documentation/security/IMA-templates.rst
> +++ b/Documentation/security/IMA-templates.rst
> @@ -68,6 +68,9 @@ descriptors by adding their identifier to the format string
>    - 'd-ng': the digest of the event, calculated with an arbitrary hash
>      algorithm (field format: [<hash algo>:]digest, where the digest
>      prefix is shown only if the hash algorithm is not SHA1 or MD5);
> + - 'd-ngv2': same as d-ng, but prefixed with the digest type.
> +    field format: [<digest type>:<hash algo>:]digest,
> +        where the digest type is either "ima" or "verity".
>    - 'd-modsig': the digest of the event without the appended modsig;
>    - 'n-ng': the name of the event, without size limitations;
>    - 'sig': the file signature, or the EVM portable signature if the file
> @@ -106,3 +109,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 c6805af46211..525b13916745 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_max_digest_data *hash)
> +{
> +	u8 verity_digest[FS_VERITY_MAX_DIGEST_SIZE];
> +	enum hash_algo verity_alg;
> +	int ret;
> +
> +	ret = fsverity_get_digest(iint->inode, verity_digest, &verity_alg);
> +	if (ret)
> +		return ret;
> +	if (hash->hdr.algo != verity_alg)
> +		return -EINVAL;
> +	hash->hdr.length = hash_digest_size[verity_alg];
> +	memcpy(hash->hdr.digest, verity_digest, hash->hdr.length);

Could you not pass hash->hdr.digest into fsverity_get_digest() and avoid 
the copying here?

> +	return 0;
> +}
> +
>   /*
>    * ima_collect_measurement - collect file measurement
>    *
> @@ -242,14 +260,30 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
>   	 */
>   	i_version = inode_query_iversion(inode);
>   	hash.hdr.algo = algo;
> +	hash.hdr.length = hash_digest_size[algo];
>   
>   	/* 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_REQUIRED) {
> +		result = ima_get_verity_digest(iint, &hash);
> +		switch (result) {
> +		case 0:
> +			break;
> +		case -ENODATA:
> +			audit_cause = "no-verity-digest";
> +			result = -EINVAL;
> +			break;
> +		case -EINVAL:
> +		default:
> +			audit_cause = "invalid-verity-digest";
> +			break;
> +		}
> +	} 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 a0f3775cbd82..c6b0454b2e25 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.
If they are not in sync I need to update my policy rule?

> + */
> +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_REQUIRED))
>   			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, "verity")) == 0)
> +				entry->flags |= IMA_VERITY_REQUIRED;
> +			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-ngv2 template field recommended for unsigned fs-verity digests */
> +	if (!result && entry->action == MEASURE &&
> +	    entry->flags & IMA_VERITY_REQUIRED) {
> +		template_desc = entry->template ? entry->template :
> +						  ima_template_desc_current();
> +		check_template_field(template_desc, "d-ngv2",
> +				     "verity rules should include d-ngv2");
> +	}
> +
>   	audit_log_format(ab, "res=%d", !result);
>   	audit_log_end(ab);
>   	return result;
> @@ -2155,6 +2189,8 @@ int ima_policy_show(struct seq_file *m, void *v)
>   		else
>   			seq_puts(m, "appraise_type=imasig ");
>   	}
> +	if (entry->flags & IMA_VERITY_REQUIRED)
> +		seq_puts(m, "digest_type=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 bd95864a5f6f..0cff6658a4c2 100644
> --- a/security/integrity/ima/ima_template_lib.c
> +++ b/security/integrity/ima/ima_template_lib.c
> @@ -31,7 +31,7 @@ enum data_formats {
>   };
>   
>   #define DIGEST_TYPE_MAXLEN 16	/* including NULL */
> -static const char * const digest_type_name[] = {"ima"};
> +static const char * const digest_type_name[] = {"ima", "verity"};
>   static int digest_type_size = ARRAY_SIZE(digest_type_name);

if this static needs to exist at all, and I dn't think it should, it 
should probably be called digest_type_array_size. Though I would just 
use ARRAY_SIZE() where needed.

>   
>   static int ima_write_template_field_data(const void *data, const u32 datalen,
> @@ -430,6 +430,8 @@ int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
>   	cur_digestsize = event_data->iint->ima_hash->length;
>   
>   	hash_algo = event_data->iint->ima_hash->algo;
> +	if (event_data->iint->flags & IMA_VERITY_REQUIRED)
> +		digest_type = 1;
>   out:
>   	return ima_eventdigest_init_common(cur_digest, cur_digestsize,
>   					   digest_type, hash_algo, field_data);
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index daf49894fd7d..d42a01903f08 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -32,7 +32,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
> @@ -40,6 +40,7 @@
>   #define IMA_FAIL_UNVERIFIABLE_SIGS	0x10000000
>   #define IMA_MODSIG_ALLOWED	0x20000000
>   #define IMA_CHECK_BLACKLIST	0x40000000
> +#define IMA_VERITY_REQUIRED	0x80000000
>   
>   #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>   				 IMA_HASH | IMA_APPRAISE_SUBMASK)

Re: [PATCH v6 4/5] ima: support fs-verity file digest based version 3 signatures

[Stefan Berger]

On 3/18/22 14:21, Mimi Zohar wrote:
> IMA may verify a file's integrity against a "good" value stored in the
> 'security.ima' xattr or as an appended signature, based on policy.  When
> the "good value" is stored in the xattr, the xattr may contain a file
> hash or signature.  In either case, the "good" value is preceded by a
> header.  The first byte of the xattr header indicates the type of data
> - hash, signature - stored in the xattr.  To support storing fs-verity
> signatures in the 'security.ima' xattr requires further differentiating
> the fs-verity signature from the existing IMA signature.
> 
> In addition the signatures stored in 'security.ima' xattr, need to be
> disambiguated.  Instead of directly signing the fs-verity digest, a new
> signature version 3 is defined as the hash of the ima_file_id structure,
> which identifies the type of signature and the digest.

Would it not be enough to just differentiat by the type of signature 
rather than also bumping the version? It's still signature_v2_hdr but a 
new type IMA_VERITY_DIGSIG is introduced there that shoud be sufficient 
to indicate that a different method for calculating the hash is to be 
used than for anything that existed before? sigv3 would then become the 
more obvious veriftysig... ?

Re: [PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates

[Mimi Zohar]

On Mon, 2022-03-21 at 08:53 -0400, Stefan Berger wrote:
> >   	/*
> >   	 * digest formats:
> >   	 *  - DATA_FMT_DIGEST: digest
> >   	 *  - DATA_FMT_DIGEST_WITH_ALGO: [<hash algo>] + ':' + '\0' + digest,
> > +	 *  - DATA_FMT_DIGEST_WITH_TYPE_AND_ALGO:
> > +	 *	[<digest type> + ':' + <hash algo>] + ':' + '\0' + digest,
> > +	 *    where <hash type> is either "ima" or "verity",
> >   	 *    where <hash algo> is provided if the hash algorithm is not
> >   	 *    SHA1 or MD5
> >   	 */
> > -	u8 buffer[CRYPTO_MAX_ALG_NAME + 2 + IMA_MAX_DIGEST_SIZE] = { 0 };
> > +	u8 buffer[DIGEST_TYPE_MAXLEN + CRYPTO_MAX_ALG_NAME + 2 +
> > +		IMA_MAX_DIGEST_SIZE] = { 0 };
> 
> Should it not be DIGEST_TYPE_MAXLEN + 1 /* for ':' */ + 
> CRYPTO_MAX_ALG_NAME + ....

DIGEST_TYPE_MAXLEN is hard coded as 16.

> 
> >   	enum data_formats fmt = DATA_FMT_DIGEST;
> >   	u32 offset = 0;
> >   
> > -	if (hash_algo < HASH_ALGO__LAST) {
> > -		fmt = DATA_FMT_DIGEST_WITH_ALGO;
> > -		offset += snprintf(buffer, CRYPTO_MAX_ALG_NAME + 1, "%s",
> > +	if (digest_type < digest_type_size && hash_algo < HASH_ALGO__LAST) {
> 
> It's a bit difficult to see what the digest_type has to do with size...
> Maybe digest_type should be a enum and the comparison here should be 
> digest_type != DIGEST_TYPE_NONE or something like it..

digest_type_size is the size of the array.  It's defined as "static int
digest_type_size = ARRAY_SIZE(digest_type_name);", with the first
element as "ima".

> > +
> > +/*
> > + * This function writes the digest of an event (without size limit),
> > + * prefixed with both the hash type and algorithm.
> > + */
> > +int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
> > +			      struct ima_field_data *field_data)
> > +{
> > +	u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
> > +	u32 cur_digestsize = 0;
> > +	u8 digest_type = 0;
> 
> What does '0' mean? I think this should definitely be an enum or at 
> least #define.

The first element of the array is "ima".  Should I define two macros
similar to kernel_read_file_id and kernel_read_file_str for just two
strings?

thanks,

Mimi

Re: [PATCH v6 3/5] ima: permit fsverity's file digests in the IMA measurement list

[Mimi Zohar]

On Mon, 2022-03-21 at 08:59 -0400, Stefan Berger wrote:

> > +/*
> > + * Make sure the policy rule and template format are in sync.
> If they are not in sync I need to update my policy rule?

It doesn't prevent loading the policy, if they're not in sync, but
simply issues a warning.

> 
> > + */
> > +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_REQUIRED))
> >   			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, "verity")) == 0)
> > +				entry->flags |= IMA_VERITY_REQUIRED;
> > +			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-ngv2 template field recommended for unsigned fs-verity digests */
> > +	if (!result && entry->action == MEASURE &&
> > +	    entry->flags & IMA_VERITY_REQUIRED) {
> > +		template_desc = entry->template ? entry->template :
> > +						  ima_template_desc_current();
> > +		check_template_field(template_desc, "d-ngv2",
> > +				     "verity rules should include d-ngv2");
> > +	}
> > +
> >   	audit_log_format(ab, "res=%d", !result);
> >   	audit_log_end(ab);
> >   	return result;
> > @@ -2155,6 +2189,8 @@ int ima_policy_show(struct seq_file *m, void *v)
> >   		else
> >   			seq_puts(m, "appraise_type=imasig ");
> >   	}
> > +	if (entry->flags & IMA_VERITY_REQUIRED)
> > +		seq_puts(m, "digest_type=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 bd95864a5f6f..0cff6658a4c2 100644
> > --- a/security/integrity/ima/ima_template_lib.c
> > +++ b/security/integrity/ima/ima_template_lib.c
> > @@ -31,7 +31,7 @@ enum data_formats {
> >   };
> >   
> >   #define DIGEST_TYPE_MAXLEN 16	/* including NULL */
> > -static const char * const digest_type_name[] = {"ima"};
> > +static const char * const digest_type_name[] = {"ima", "verity"};
> >   static int digest_type_size = ARRAY_SIZE(digest_type_name);
> 
> if this static needs to exist at all, and I dn't think it should, it 
> should probably be called digest_type_array_size. Though I would just 
> use ARRAY_SIZE() where needed.

Ok.

thanks,

Mimi

Re: [PATCH v6 2/5] ima: define a new template field named 'd-ngv2' and templates

[Stefan Berger]

On 3/21/22 15:48, Mimi Zohar wrote:
> On Mon, 2022-03-21 at 08:53 -0400, Stefan Berger wrote:

>>> +
>>> +/*
>>> + * This function writes the digest of an event (without size limit),
>>> + * prefixed with both the hash type and algorithm.
>>> + */
>>> +int ima_eventdigest_ngv2_init(struct ima_event_data *event_data,
>>> +			      struct ima_field_data *field_data)
>>> +{
>>> +	u8 *cur_digest = NULL, hash_algo = HASH_ALGO_SHA1;
>>> +	u32 cur_digestsize = 0;
>>> +	u8 digest_type = 0;
>>
>> What does '0' mean? I think this should definitely be an enum or at
>> least #define.
> 
> The first element of the array is "ima".  Should I define two macros
> similar to kernel_read_file_id and kernel_read_file_str for just two
> strings?

I would introduce an enum like enum hash_algo: 
https://elixir.bootlin.com/linux/latest/source/include/uapi/linux/hash_info.h#L38

And an array like hash_algo_name: 
https://elixir.bootlin.com/linux/latest/source/crypto/hash_info.c#L12


> 
> thanks,
> 
> Mimi
> 

Re: [PATCH v6 4/5] ima: support fs-verity file digest based version 3 signatures

[Mimi Zohar]

On Mon, 2022-03-21 at 09:10 -0400, Stefan Berger wrote:
> 
> On 3/18/22 14:21, Mimi Zohar wrote:
> > IMA may verify a file's integrity against a "good" value stored in the
> > 'security.ima' xattr or as an appended signature, based on policy.  When
> > the "good value" is stored in the xattr, the xattr may contain a file
> > hash or signature.  In either case, the "good" value is preceded by a
> > header.  The first byte of the xattr header indicates the type of data
> > - hash, signature - stored in the xattr.  To support storing fs-verity
> > signatures in the 'security.ima' xattr requires further differentiating
> > the fs-verity signature from the existing IMA signature.
> > 
> > In addition the signatures stored in 'security.ima' xattr, need to be
> > disambiguated.  Instead of directly signing the fs-verity digest, a new
> > signature version 3 is defined as the hash of the ima_file_id structure,
> > which identifies the type of signature and the digest.
> 
> Would it not be enough to just differentiat by the type of signature 
> rather than also bumping the version? It's still signature_v2_hdr but a 
> new type IMA_VERITY_DIGSIG is introduced there that shoud be sufficient 
> to indicate that a different method for calculating the hash is to be 
> used than for anything that existed before? sigv3 would then become the 
> more obvious veriftysig... ?

One of Eric's concerns was that, "an attacker (who controls the file's
contents and IMA xattr) [could] replace the file with one with a
differrent content and still be able to pass the IMA check."  His
solution was to only allow one signature version on a running system.  
For the complete description of the attack, refer to Eric's comments on
v3.

Instead of only allowing one signature version on a running system,
subsequent versions of this patch set addressed his concern, by
limiting the signature version based on policy.

-- 
thanks,

Mimi

Re: [PATCH v6 4/5] ima: support fs-verity file digest based version 3 signatures

[Stefan Berger]

On 3/25/22 08:31, Mimi Zohar wrote:
> On Mon, 2022-03-21 at 09:10 -0400, Stefan Berger wrote:
>>
>> On 3/18/22 14:21, Mimi Zohar wrote:
>>> IMA may verify a file's integrity against a "good" value stored in the
>>> 'security.ima' xattr or as an appended signature, based on policy.  When
>>> the "good value" is stored in the xattr, the xattr may contain a file
>>> hash or signature.  In either case, the "good" value is preceded by a
>>> header.  The first byte of the xattr header indicates the type of data
>>> - hash, signature - stored in the xattr.  To support storing fs-verity
>>> signatures in the 'security.ima' xattr requires further differentiating
>>> the fs-verity signature from the existing IMA signature.
>>>
>>> In addition the signatures stored in 'security.ima' xattr, need to be
>>> disambiguated.  Instead of directly signing the fs-verity digest, a new
>>> signature version 3 is defined as the hash of the ima_file_id structure,
>>> which identifies the type of signature and the digest.
>>
>> Would it not be enough to just differentiat by the type of signature
>> rather than also bumping the version? It's still signature_v2_hdr but a
>> new type IMA_VERITY_DIGSIG is introduced there that shoud be sufficient
>> to indicate that a different method for calculating the hash is to be
>> used than for anything that existed before? sigv3 would then become the
>> more obvious veriftysig... ?
> 
> One of Eric's concerns was that, "an attacker (who controls the file's
> contents and IMA xattr) [could] replace the file with one with a

Reference: 
https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/T/#m8929fa29fbdfc875dbf5f384a4c082d303d2040e

This seem to describe the root user. A restrictions of root's power 
maybe that root may not have access to the file signing key use on the 
local system... ?

> differrent content and still be able to pass the IMA check."  His

Is this a scenario of concern? : /usr/bin/foobar is signed by verity and 
there's a rule in the IMA policy that would appraise this file. Can root 
now remove /usr/bin/foobar and copy the regularly signed /usr/bin/bash 
to /usr/bin/foobar along with bash's security.ima and have it execute 
either since there's no appraise rule covering non-fsverity signatures 
or due to a rule that covers non-fsverity signatures?

Since the signature header of security.ima is not signed root could also 
just rewrite the header and modify the signature type (and also version) 
and circumvent appraisal rules specific to fsverity.

> solution was to only allow one signature version on a running system.
> For the complete description of the attack, refer to Eric's comments on
> v3.


I am trying to figure out a concrete scenario that one has to defend 
against what seems to be the power of the root user. A more concrete 
example may be helpful.

> 
> Instead of only allowing one signature version on a running system,
> subsequent versions of this patch set addressed his concern, by
> limiting the signature version based on policy.
>