[RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Roberto Sassu]

I wanted to propose a different approach for handling fsverity digests and
signatures, compared to:

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

In the original proposal, a new signature version has been introduced (v3)
to allow the possibility of signing the digest of a more flexible data
structure, ima_file_id, which could also include the fsverity file digest.

While the new signature type would be sufficient to handle fsverity file
digests, the problem is that its format would not be compatible with the
signature format supported by the built-in verification module in fsverity.
The rpm package manager already has an extension to include fsverity
signatures, with the existing format, in the RPM header.

Given that the fsverity signature is in the PKCS#7 format, IMA has already
the capability of handling it with the existing code, more specifically the
modsig code. It would be sufficient to provide to modsig the correct data
to avoid introducing a new signature format.

This is what this alternative patch set does. Patches 1-5, 8 have been
omitted as they almost don't need modification. Patches 6-7 of this patch
set extend the fsverity API to get the necessary information to handle the
existing fsverity signatures. Patch 8 (which could be split in two parts,
moving the appraisal-specific part to a new patch) gets the fsverity
formatted digest and the signature, if present, and use the obtained
information for measurement, appraisal and audit.

Interference with the code dealing with modsig has been elimitated by
introducing the new function ima_modsig_is_verity(), from which that
code knows how to deal with the data structure.

Also, the fsverity method needs to be enabled with the policy (no change
from the original patch set) and is used only if the xattr and modsig
appraisal methods are not available.

Regarding the measurement part, the original patch set avoids the ambiguity
of d-ng, or with the new template field d-type, or with the new signature
type IMA_XATTR_DIGSIG in the sig field. This patch set removes the
ambiguity by linking d-ng with d-modsig: if d-modsig is the digest of the
formatted digest including d-ng, sig is an fsverity signature, otherwise it
is a modsig signature.

Finally, this patch set addresses also the EVM part. Since the link of an
EVM portable signature/HMAC is not done anymore with the IMA xattr, as in
the original patch set, EVM directly fetches the formatted digest from
fsverity, and includes it in the HMAC/digest calculation. This behavior is
disabled by default and needs to be enabled in the kernel configuration.
A new function has been exposed to tell to IMA whether or not the fsverity
formatted digest is protected.

Remaining work would probably be to introduce new template fields to
specifically store the fsverity formatted digest and signature (instead of
d-modsig and modsig).

Mimi Zohar (6):
  ima: rename IMA_ACTION_FLAGS to IMA_NONACTION_FLAGS
  ima: define ima_max_digest_data struct without a flexible array
    variable
  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
  fsverity: update the documentation

Roberto Sassu (5):
  fsverity: Introduce fsverity_get_formatted_digest()
  fsverity: Introduce fsverity_get_signature()
  fsverity: Completely disable signature verification if not requested
  ima: Add support for fsverity signatures
  evm: Include fsverity formatted digest in the HMAC/digest calculation

 Documentation/ABI/testing/ima_policy      |  17 +++
 Documentation/filesystems/fsverity.rst    |  22 ++--
 Documentation/security/IMA-templates.rst  |  13 ++-
 fs/verity/Kconfig                         |   1 +
 fs/verity/fsverity_private.h              |   7 --
 fs/verity/measure.c                       | 123 ++++++++++++++++++++++
 fs/verity/signature.c                     |  12 +--
 include/linux/evm.h                       |   9 ++
 include/linux/fsverity.h                  |  37 +++++++
 security/integrity/evm/Kconfig            |  15 +++
 security/integrity/evm/evm_crypto.c       |  18 ++++
 security/integrity/evm/evm_main.c         |   4 +
 security/integrity/ima/ima.h              |  21 +++-
 security/integrity/ima/ima_api.c          |  19 +++-
 security/integrity/ima/ima_appraise.c     |  67 ++++++++++--
 security/integrity/ima/ima_crypto.c       |   2 +-
 security/integrity/ima/ima_init.c         |   9 +-
 security/integrity/ima/ima_main.c         |  34 +++++-
 security/integrity/ima/ima_modsig.c       |  75 +++++++++++++
 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            |  30 +++++-
 24 files changed, 553 insertions(+), 50 deletions(-)

-- 
2.32.0

[RFC][PATCH v3a 06/11] fsverity: Introduce fsverity_get_formatted_digest()

[Roberto Sassu]

Introduce fsverity_get_formatted_digest() so that a caller (e.g. IMA) can
get all the information necessary to validate the fsverity signature.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/measure.c      | 46 ++++++++++++++++++++++++++++++++++++++++
 include/linux/fsverity.h | 12 +++++++++++
 2 files changed, 58 insertions(+)

diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index 2152f115071a..7afe4274ecb0 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -8,6 +8,7 @@
 #include "fsverity_private.h"
 
 #include <linux/uaccess.h>
+#include <linux/slab.h>
 
 /**
  * fsverity_ioctl_measure() - get a verity file's digest
@@ -96,3 +97,48 @@ int fsverity_get_digest(struct inode *inode,
 
 	return 0;
 }
+
+/**
+ * fsverity_get_formatted_digest() - get a verity file's formatted digest
+ * @inode: inode to get the formatted digest from
+ * @formatted_digest: (out) pointer to the formatted digest
+ * @alg: (out) pointer to the hash algorithm enumeration
+ *
+ * Return the fsverity_formatted_digest structure for a given inode.
+ *
+ * Return: written bytes on success, -errno on failure
+ */
+ssize_t fsverity_get_formatted_digest(struct inode *inode,
+			u8 formatted_digest[FS_VERITY_MAX_FMT_DIGEST_SIZE],
+			enum hash_algo *alg)
+{
+	struct fsverity_formatted_digest *d =
+			(struct fsverity_formatted_digest *)formatted_digest;
+	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;
+
+	/* 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;
+
+	memset(formatted_digest, 0, FS_VERITY_MAX_FMT_DIGEST_SIZE);
+
+	memcpy(d->magic, "FSVerity", 8);
+	d->digest_algorithm = cpu_to_le16(hash_alg - fsverity_hash_algs);
+	d->digest_size = cpu_to_le16(hash_alg->digest_size);
+	memcpy(d->digest, vi->file_digest, hash_alg->digest_size);
+
+	pr_debug("file formatted digest FSVerity:%s:%d:%*phN\n", hash_alg->name,
+		 hash_alg->digest_size, hash_alg->digest_size, vi->file_digest);
+
+	return sizeof(*d) + hash_alg->digest_size;
+}
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 9a1b70cc7318..17ae313ed8f4 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -21,6 +21,8 @@
  * Currently assumed to be <= size of fsverity_descriptor::root_hash.
  */
 #define FS_VERITY_MAX_DIGEST_SIZE	SHA512_DIGEST_SIZE
+#define FS_VERITY_MAX_FMT_DIGEST_SIZE	(FS_VERITY_MAX_DIGEST_SIZE + \
+				sizeof(struct fsverity_formatted_digest))
 
 /* Verity operations for filesystems */
 struct fsverity_operations {
@@ -142,6 +144,9 @@ 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);
+ssize_t fsverity_get_formatted_digest(struct inode *inode,
+			u8 formatted_digest[FS_VERITY_MAX_FMT_DIGEST_SIZE],
+			enum hash_algo *alg);
 
 /* open.c */
 
@@ -188,6 +193,13 @@ static inline int fsverity_get_digest(struct inode *inode,
 	return -EOPNOTSUPP;
 }
 
+static inline ssize_t fsverity_get_formatted_digest(struct inode *inode,
+			u8 formatted_digest[FS_VERITY_MAX_FMT_DIGEST_SIZE],
+			enum hash_algo *alg)
+{
+	return -EOPNOTSUPP;
+}
+
 /* open.c */
 
 static inline int fsverity_file_open(struct inode *inode, struct file *filp)
-- 
2.32.0

[RFC][PATCH v3a 07/11] fsverity: Introduce fsverity_get_signature()

[Roberto Sassu]

Get the signature of an fsverity-protected file.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/measure.c      | 38 ++++++++++++++++++++++++++++++++++++++
 include/linux/fsverity.h |  7 +++++++
 2 files changed, 45 insertions(+)

diff --git a/fs/verity/measure.c b/fs/verity/measure.c
index 7afe4274ecb0..679e2ddae62c 100644
--- a/fs/verity/measure.c
+++ b/fs/verity/measure.c
@@ -142,3 +142,41 @@ ssize_t fsverity_get_formatted_digest(struct inode *inode,
 
 	return sizeof(*d) + hash_alg->digest_size;
 }
+
+/**
+ * fsverity_get_signature() - get a verity file's signature
+ * @inode: inode to get signature of
+ * @signature: (out) pointer to the signature
+ *
+ * Return the file signature of an fsverity-protected file.
+ *
+ * Return: written bytes on success, -errno on failure
+ */
+ssize_t fsverity_get_signature(struct inode *inode, u8 **signature)
+{
+	const struct fsverity_info *vi;
+	struct fsverity_descriptor *desc;
+	size_t desc_size;
+	int err, signature_size;
+
+	vi = fsverity_get_info(inode);
+	if (!vi)
+		return -ENODATA; /* not a verity file */
+
+	err = fsverity_get_descriptor(inode, &desc, &desc_size);
+	if (err)
+		return err;
+
+	signature_size = le32_to_cpu(desc->sig_size);
+
+	*signature = kmemdup(desc->signature, signature_size, GFP_KERNEL);
+
+	kfree(desc);
+
+	if (!*signature)
+		return -ENOMEM;
+
+	pr_debug("file signature %*phN\n", signature_size, *signature);
+
+	return signature_size;
+}
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 17ae313ed8f4..5ad7921f3589 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -147,6 +147,7 @@ int fsverity_get_digest(struct inode *inode,
 ssize_t fsverity_get_formatted_digest(struct inode *inode,
 			u8 formatted_digest[FS_VERITY_MAX_FMT_DIGEST_SIZE],
 			enum hash_algo *alg);
+ssize_t fsverity_get_signature(struct inode *inode, u8 **signature);
 
 /* open.c */
 
@@ -200,6 +201,12 @@ static inline ssize_t fsverity_get_formatted_digest(struct inode *inode,
 	return -EOPNOTSUPP;
 }
 
+static inline ssize_t fsverity_get_signature(struct inode *inode,
+					     u8 **signature)
+{
+	return -EOPNOTSUPP;
+}
+
 /* open.c */
 
 static inline int fsverity_file_open(struct inode *inode, struct file *filp)
-- 
2.32.0

[RFC][PATCH v3a 08/11] fsverity: Completely disable signature verification if not requested

[Roberto Sassu]

Currently, fsverity verifies the signature, if supplied, regardless of
whether signature verification is requested or not.

Completely disable signature verification, if not requested, so that other
users of fsverity can do their own verification without relying on the
fsverity-specific verification to work.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/verity/signature.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index 143a530a8008..b45a2cea6c59 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -45,13 +45,13 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
 	struct fsverity_formatted_digest *d;
 	int err;
 
-	if (sig_size == 0) {
-		if (fsverity_require_signatures) {
-			fsverity_err(inode,
-				     "require_signatures=1, rejecting unsigned file!");
-			return -EPERM;
-		}
+	if (!fsverity_require_signatures)
 		return 0;
+
+	if (sig_size == 0) {
+		fsverity_err(inode,
+			     "require_signatures=1, rejecting unsigned file!");
+		return -EPERM;
 	}
 
 	d = kzalloc(sizeof(*d) + hash_alg->digest_size, GFP_KERNEL);
-- 
2.32.0

[RFC][PATCH v3a 09/11] ima: Add support for fsverity signatures

[Roberto Sassu]

Since fsverity signatures are in PKCS#7 format, handle them as the same as
kernel modules, using the modsig code.

The main differences with modsig are: ima_read_fsverity_sig() gets the
fsverity signature with fsverity_get_signature() instead of getting it from
the file content; ima_collect_fsverity() gets the data to be hashed from
fsverity_get_formatted_digest(), instead of hashing the file content.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/ABI/testing/ima_policy     | 10 ++++
 Documentation/security/IMA-templates.rst |  7 ++-
 include/linux/evm.h                      |  5 ++
 security/integrity/ima/ima.h             | 19 ++++++
 security/integrity/ima/ima_api.c         | 38 ++++--------
 security/integrity/ima/ima_appraise.c    | 67 ++++++++++++++++++---
 security/integrity/ima/ima_main.c        | 32 ++++++++++
 security/integrity/ima/ima_modsig.c      | 75 ++++++++++++++++++++++++
 8 files changed, 217 insertions(+), 36 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 444bb7ccbe03..8602f08d06bb 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-modsig
+			appraise func=BPRM_CHECK digest_type=hash|verity \
+				ima_appraise_type=imasig 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..96654e72a36e 100644
--- a/Documentation/security/IMA-templates.rst
+++ b/Documentation/security/IMA-templates.rst
@@ -68,11 +68,12 @@ 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-modsig': the digest of the event without the appended modsig;
+ - 'd-modsig': the digest of the event without the appended modsig, or the
+   digest of an fsverity formatted digest;
  - '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/linux/evm.h b/include/linux/evm.h
index 4c374be70247..3da25393b011 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -14,6 +14,11 @@
 
 struct integrity_iint_cache;
 
+static inline bool evm_protects_fsverity(void)
+{
+	return false;
+}
+
 #ifdef CONFIG_EVM
 extern int evm_set_key(void *key, size_t keylen);
 extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 78395bed7fad..4a45a7b5743b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -380,26 +380,45 @@ static inline int ima_read_xattr(struct dentry *dentry,
 #endif /* CONFIG_IMA_APPRAISE */
 
 #ifdef CONFIG_IMA_APPRAISE_MODSIG
+bool ima_modsig_is_verity(const struct modsig *modsig);
 int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
 		    struct modsig **modsig);
+int ima_read_fsverity_sig(struct inode *inode, struct modsig **modsig);
 void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size);
+void ima_collect_fsverity(struct modsig *modsig, const void *buf, loff_t size);
 int ima_get_modsig_digest(const struct modsig *modsig, enum hash_algo *algo,
 			  const u8 **digest, u32 *digest_size);
 int ima_get_raw_modsig(const struct modsig *modsig, const void **data,
 		       u32 *data_len);
 void ima_free_modsig(struct modsig *modsig);
 #else
+static inline bool ima_modsig_is_verity(const struct modsig *modsig)
+{
+	return false;
+}
+
 static inline int ima_read_modsig(enum ima_hooks func, const void *buf,
 				  loff_t buf_len, struct modsig **modsig)
 {
 	return -EOPNOTSUPP;
 }
 
+static inline int ima_read_fsverity_sig(struct inode *inode,
+					struct modsig **modsig)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline void ima_collect_modsig(struct modsig *modsig, const void *buf,
 				      loff_t size)
 {
 }
 
+static inline void ima_collect_fsverity(struct modsig *modsig, const void *buf,
+					loff_t size)
+{
+}
+
 static inline int ima_get_modsig_digest(const struct modsig *modsig,
 					enum hash_algo *algo, const u8 **digest,
 					u32 *digest_size)
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 8760c4874f7d..369f2222dd55 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -201,23 +201,6 @@ 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
  *
@@ -249,8 +232,12 @@ int ima_collect_measurement(struct integrity_iint_cache *iint,
 	 * the file digest without collecting the modsig in a previous
 	 * measurement rule.
 	 */
-	if (modsig)
-		ima_collect_modsig(modsig, buf, size);
+	if (modsig) {
+		if (!ima_modsig_is_verity(modsig))
+			ima_collect_modsig(modsig, buf, size);
+		else
+			ima_collect_fsverity(modsig, buf, size);
+	}
 
 	if (iint->flags & IMA_COLLECTED)
 		goto out;
@@ -266,14 +253,13 @@ 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 && !(iint->flags & IMA_VERITY_DIGEST)) {
 		result = ima_calc_buffer_hash(buf, size, &hash.hdr);
-	} 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 if (buf && (iint->flags & IMA_VERITY_DIGEST)) {
+		hash.hdr.length = hash_digest_size[algo];
+		memcpy(hash.hdr.digest,
+		       ((struct fsverity_formatted_digest *)buf)->digest,
+		       hash_digest_size[algo]);
 	} else {
 		result = ima_calc_file_hash(file, &hash.hdr);
 	}
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 17232bbfb9f9..f8dde59e64f5 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -320,7 +320,7 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
 
 	rc = integrity_modsig_verify(INTEGRITY_KEYRING_IMA, modsig);
 	if (IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING) && rc &&
-	    func == KEXEC_KERNEL_CHECK)
+	    func == KEXEC_KERNEL_CHECK && !ima_modsig_is_verity(modsig))
 		rc = integrity_modsig_verify(INTEGRITY_KEYRING_PLATFORM,
 					     modsig);
 	if (rc) {
@@ -333,6 +333,50 @@ static int modsig_verify(enum ima_hooks func, const struct modsig *modsig,
 	return rc;
 }
 
+/*
+ * fsverity_verify - verify fsverity signature
+ *
+ * Verify whether the fsverity signature is valid.
+ *
+ * Return 0 on success, error code otherwise.
+ */
+static int fsverity_verify(struct integrity_iint_cache *iint,
+			   const struct modsig *modsig,
+			   enum integrity_status *status, const char **cause)
+{
+	int rc = -EINVAL;
+
+	if (!modsig || !ima_modsig_is_verity(modsig)) {
+		if (!evm_protects_fsverity()) {
+			*cause = "EVM-fsverity-not-protected";
+			*status = INTEGRITY_FAIL;
+			return rc;
+		}
+
+		if (*status != INTEGRITY_PASS_IMMUTABLE) {
+			if (iint->flags & IMA_DIGSIG_REQUIRED) {
+				*cause = "IMA-signature-required";
+				*status = INTEGRITY_FAIL;
+				return rc;
+			}
+			clear_bit(IMA_DIGSIG, &iint->atomic_flags);
+		} else {
+			set_bit(IMA_DIGSIG, &iint->atomic_flags);
+		}
+
+		/*
+		 * EVM already verified the actual fsverity digest, nothing else
+		 * is required.
+		 */
+		*status = INTEGRITY_PASS;
+		rc = 0;
+	} else {
+		rc = modsig_verify(NONE, modsig, status, cause);
+	}
+
+	return rc;
+}
+
 /*
  * ima_check_blacklist - determine if the binary is blacklisted.
  *
@@ -352,7 +396,8 @@ int ima_check_blacklist(struct integrity_iint_cache *iint,
 	if (!(iint->flags & IMA_CHECK_BLACKLIST))
 		return 0;
 
-	if (iint->flags & IMA_MODSIG_ALLOWED && modsig) {
+	if (iint->flags & IMA_MODSIG_ALLOWED && modsig &&
+	    !ima_modsig_is_verity(modsig)) {
 		ima_get_modsig_digest(modsig, &hash_algo, &digest, &digestsize);
 
 		rc = is_binary_blacklisted(digest, digestsize);
@@ -385,14 +430,16 @@ int ima_appraise_measurement(enum ima_hooks func,
 	struct inode *inode = d_backing_inode(dentry);
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len;
-	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
+	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig &&
+			  !ima_modsig_is_verity(modsig);
+	bool try_fsverity = iint->flags & IMA_VERITY_DIGEST;
 
-	/* If not appraising a modsig, we need an xattr. */
-	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
+	/* If not appraising a modsig or an fsverity file, we need an xattr. */
+	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !try_fsverity)
 		return INTEGRITY_UNKNOWN;
 
 	/* If reading the xattr failed and there's no modsig, error out. */
-	if (rc <= 0 && !try_modsig) {
+	if (rc <= 0 && !try_modsig && !try_fsverity) {
 		if (rc && rc != -ENODATA)
 			goto out;
 
@@ -446,6 +493,12 @@ int ima_appraise_measurement(enum ima_hooks func,
 	     rc == -ENOKEY))
 		rc = modsig_verify(func, modsig, &status, &cause);
 
+	/*
+	 * If we have a fsverity sig, no modsig and no imasig, then try
+	 * verifying the fsverity sig.
+	 */
+	if (try_fsverity)
+		rc = fsverity_verify(iint, modsig, &status, &cause);
 out:
 	/*
 	 * File signatures on some filesystems can not be properly verified.
@@ -463,7 +516,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	} else if (status != INTEGRITY_PASS) {
 		/* Fix mode, but don't replace file signatures. */
 		if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
-		    (!xattr_value ||
+		    !try_fsverity && (!xattr_value ||
 		     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
 			if (!ima_fix_xattr(dentry, iint))
 				status = INTEGRITY_PASS;
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 774accb62275..1f78f31c3e89 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -25,6 +25,7 @@
 #include <linux/xattr.h>
 #include <linux/ima.h>
 #include <linux/iversion.h>
+#include <linux/fsverity.h>
 #include <linux/fs.h>
 
 #include "ima.h"
@@ -216,6 +217,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	bool violation_check;
 	enum hash_algo hash_algo;
 	unsigned int allowed_algos = 0;
+	u8 fsverity_buf[FS_VERITY_MAX_FMT_DIGEST_SIZE];
+	ssize_t fsverity_buf_len;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -330,9 +333,38 @@ static int process_measurement(struct file *file, const struct cred *cred,
 			    iint->flags & IMA_MEASURED)
 				action |= IMA_MEASURE;
 		}
+
+		/*
+		 * Read the fsverity sig if allowed by the policy, and allow
+		 * an additional measurement list entry, if needed, based on the
+		 * template format and whether the file was already measured.
+		 */
+		if (!modsig && IS_VERITY(inode) &&
+		    (iint->flags & IMA_VERITY_ALLOWED)) {
+			rc = ima_read_fsverity_sig(inode, &modsig);
+
+			if (!rc && ima_template_has_modsig(template_desc) &&
+			    iint->flags & IMA_MEASURED)
+				action |= IMA_MEASURE;
+		}
 	}
 
 	hash_algo = ima_get_hash_algo(xattr_value, xattr_len);
+	/*
+	 * Fsverity verification method is enabled only if all others are not
+	 * available.
+	 */
+	if (IS_VERITY(inode) && (iint->flags & IMA_VERITY_ALLOWED) &&
+	    !xattr_value && (!modsig || ima_modsig_is_verity(modsig))) {
+		fsverity_buf_len = fsverity_get_formatted_digest(inode,
+								 fsverity_buf,
+								 &hash_algo);
+		if (fsverity_buf_len > 0) {
+			buf = fsverity_buf;
+			size = fsverity_buf_len;
+			iint->flags |= IMA_VERITY_DIGEST;
+		}
+	}
 
 	rc = ima_collect_measurement(iint, file, buf, size, hash_algo, modsig);
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
diff --git a/security/integrity/ima/ima_modsig.c b/security/integrity/ima/ima_modsig.c
index fb25723c65bc..66c19846477c 100644
--- a/security/integrity/ima/ima_modsig.c
+++ b/security/integrity/ima/ima_modsig.c
@@ -9,6 +9,7 @@
  */
 
 #include <linux/types.h>
+#include <linux/fsverity.h>
 #include <linux/module_signature.h>
 #include <keys/asymmetric-type.h>
 #include <crypto/pkcs7.h>
@@ -16,6 +17,8 @@
 #include "ima.h"
 
 struct modsig {
+	bool is_verity;
+
 	struct pkcs7_message *pkcs7_msg;
 
 	enum hash_algo hash_algo;
@@ -32,6 +35,11 @@ struct modsig {
 	u8 raw_pkcs7[];
 };
 
+bool ima_modsig_is_verity(const struct modsig *modsig)
+{
+	return modsig->is_verity;
+}
+
 /*
  * ima_read_modsig - Read modsig from buf.
  *
@@ -87,6 +95,51 @@ int ima_read_modsig(enum ima_hooks func, const void *buf, loff_t buf_len,
 	return 0;
 }
 
+/*
+ * ima_read_fsverity_sig - Read fsverity sig from inode.
+ *
+ * Return: 0 on success, error code otherwise.
+ */
+int ima_read_fsverity_sig(struct inode *inode, struct modsig **modsig)
+{
+	struct modsig *hdr;
+	u8 *signature;
+	ssize_t signature_size;
+	int rc;
+
+	signature_size = fsverity_get_signature(inode, &signature);
+	if (signature_size < 0)
+		return signature_size;
+
+	/*
+	 * Allocate signature_size additional bytes to hold the raw PKCS#7 data.
+	 */
+	hdr = kzalloc(sizeof(*hdr) + signature_size, GFP_KERNEL);
+	if (!hdr) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	hdr->pkcs7_msg = pkcs7_parse_message(signature, signature_size);
+	if (IS_ERR(hdr->pkcs7_msg)) {
+		rc = PTR_ERR(hdr->pkcs7_msg);
+		kfree(hdr);
+		goto out;
+	}
+
+	memcpy(hdr->raw_pkcs7, signature, signature_size);
+	hdr->raw_pkcs7_len = signature_size;
+
+	/* We don't know the hash algorithm yet. */
+	hdr->hash_algo = HASH_ALGO__LAST;
+	hdr->is_verity = true;
+
+	*modsig = hdr;
+out:
+	kfree(signature);
+	return rc;
+}
+
 /**
  * ima_collect_modsig - Calculate the file hash without the appended signature.
  *
@@ -113,6 +166,28 @@ void ima_collect_modsig(struct modsig *modsig, const void *buf, loff_t size)
 			      &modsig->digest_size, &modsig->hash_algo);
 }
 
+/**
+ * ima_collect_fsverity - Calculate the digest of the fsverity formatted digest.
+ *
+ * Pass the same data used to verify the fsverity signature in
+ * fs/verity/signature.c.
+ */
+void ima_collect_fsverity(struct modsig *modsig, const void *buf, loff_t size)
+{
+	int rc;
+
+	rc = pkcs7_supply_detached_data(modsig->pkcs7_msg, buf, size);
+	if (rc)
+		return;
+
+	/*
+	 * Ask the PKCS7 code to calculate the digest of the fsverity formatted
+	 * digest.
+	 */
+	rc = pkcs7_get_digest(modsig->pkcs7_msg, &modsig->digest,
+			      &modsig->digest_size, &modsig->hash_algo);
+}
+
 int ima_modsig_verify(struct key *keyring, const struct modsig *modsig)
 {
 	return verify_pkcs7_message_sig(NULL, 0, modsig->pkcs7_msg, keyring,
-- 
2.32.0

[RFC][PATCH v3a 10/11] evm: Include fsverity formatted digest in the HMAC/digest calculation

[Roberto Sassu]

Include the fsverity formatted digest in the HMAC/diget calculation. It can
be a substitute of the IMA xattr for binding the EVM HMAC/signature to the
file content.

This feature is disabled by default, and must be enabled in the kernel
configuration.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 include/linux/evm.h                 |  4 ++++
 security/integrity/evm/Kconfig      | 15 +++++++++++++++
 security/integrity/evm/evm_crypto.c | 18 ++++++++++++++++++
 security/integrity/evm/evm_main.c   |  4 ++++
 4 files changed, 41 insertions(+)

diff --git a/include/linux/evm.h b/include/linux/evm.h
index 3da25393b011..e6637dfb22fe 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -16,7 +16,11 @@ struct integrity_iint_cache;
 
 static inline bool evm_protects_fsverity(void)
 {
+#ifdef CONFIG_EVM_ATTR_FSVERITY
+	return true;
+#else
 	return false;
+#endif
 }
 
 #ifdef CONFIG_EVM
diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index a6e19d23e700..837308dacede 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -27,6 +27,21 @@ config EVM_ATTR_FSUUID
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
+config EVM_ATTR_FSVERITY
+	bool "Include fsverity formatted digest"
+	default n
+	depends on EVM
+	depends on FS_VERITY
+	help
+	  Include fsverity formatted digest for HMAC/digest calculation.
+
+	  Default value is 'not selected'.
+
+	  WARNING: changing the HMAC/digest calculation method or adding
+	  additional info to the calculation, requires existing EVM
+	  labeled file systems to be relabeled, and the signatures to be
+	  replaced.
+
 config EVM_EXTRA_SMACK_XATTRS
 	bool "Additional SMACK xattrs"
 	depends on EVM && SECURITY_SMACK
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 0450d79afdc8..5da427d8b2c7 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -16,6 +16,7 @@
 #include <linux/crypto.h>
 #include <linux/xattr.h>
 #include <linux/evm.h>
+#include <linux/fsverity.h>
 #include <keys/encrypted-type.h>
 #include <crypto/hash.h>
 #include <crypto/hash_info.h>
@@ -224,6 +225,9 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 	int error;
 	int size, user_space_size;
 	bool ima_present = false;
+	u8 fsverity_fmt_digest[FS_VERITY_MAX_FMT_DIGEST_SIZE];
+	ssize_t fsverity_fmt_digest_len;
+	enum hash_algo fsverity_algo;
 
 	if (!(inode->i_opflags & IOP_XATTR) ||
 	    inode->i_sb->s_user_ns != &init_user_ns)
@@ -296,6 +300,20 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 			dump_security_xattr(xattr->name, xattr_value,
 					    xattr_size);
 	}
+
+	if (IS_ENABLED(CONFIG_EVM_ATTR_FSVERITY)) {
+		fsverity_fmt_digest_len = fsverity_get_formatted_digest(inode,
+							fsverity_fmt_digest,
+							&fsverity_algo);
+		if (fsverity_fmt_digest_len > 0) {
+			crypto_shash_update(desc, fsverity_fmt_digest,
+					    fsverity_fmt_digest_len);
+			/* Fsverity formatted digest satisfies this req. */
+			ima_present = true;
+			error = 0;
+		}
+	}
+
 	hmac_add_misc(desc, inode, type, data->digest);
 
 	/* Portable EVM signatures must include an IMA hash */
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 08f907382c61..8943bf4abc62 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -108,6 +108,10 @@ static void __init evm_init_config(void)
 #ifdef CONFIG_EVM_ATTR_FSUUID
 	evm_hmac_attrs |= EVM_ATTR_FSUUID;
 #endif
+
+	if (IS_ENABLED(CONFIG_EVM_ATTR_FSVERITY))
+		pr_info("Fsverity formatted digest included in calculation\n");
+
 	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
 }
 
-- 
2.32.0

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Eric Biggers]

On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> I wanted to propose a different approach for handling fsverity digests and
> signatures, compared to:
> 
> https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/
> 
> In the original proposal, a new signature version has been introduced (v3)
> to allow the possibility of signing the digest of a more flexible data
> structure, ima_file_id, which could also include the fsverity file digest.
> 
> While the new signature type would be sufficient to handle fsverity file
> digests, the problem is that its format would not be compatible with the
> signature format supported by the built-in verification module in fsverity.
> The rpm package manager already has an extension to include fsverity
> signatures, with the existing format, in the RPM header.
> 
> Given that the fsverity signature is in the PKCS#7 format, IMA has already
> the capability of handling it with the existing code, more specifically the
> modsig code. It would be sufficient to provide to modsig the correct data
> to avoid introducing a new signature format.

I think it would be best to get people moved off of the fs-verity built-in
signatures, rather than further extend the use of it.  PKCS#7 is a pretty
terrible signature format.  The IMA one is better, though it's unfortunate that
IMA still relies on X.509 for keys.

- Eric

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Eric Biggers]

On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > I wanted to propose a different approach for handling fsverity digests and
> > signatures, compared to:
> > 
> > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-zohar@linux.ibm.com/
> > 
> > In the original proposal, a new signature version has been introduced (v3)
> > to allow the possibility of signing the digest of a more flexible data
> > structure, ima_file_id, which could also include the fsverity file digest.
> > 
> > While the new signature type would be sufficient to handle fsverity file
> > digests, the problem is that its format would not be compatible with the
> > signature format supported by the built-in verification module in fsverity.
> > The rpm package manager already has an extension to include fsverity
> > signatures, with the existing format, in the RPM header.
> > 
> > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > the capability of handling it with the existing code, more specifically the
> > modsig code. It would be sufficient to provide to modsig the correct data
> > to avoid introducing a new signature format.
> 
> I think it would be best to get people moved off of the fs-verity built-in
> signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> terrible signature format.  The IMA one is better, though it's unfortunate that
> IMA still relies on X.509 for keys.

Note, the only reason that support for fs-verity built-in signatures was added
to RPM is that people didn't want to use IMA:
https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-b46a6880824b@gmail.com

If people are going to use IMA anyway, then there would be no point.

- Eric

RE: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Thursday, January 27, 2022 8:40 PM
> On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > I wanted to propose a different approach for handling fsverity digests and
> > > signatures, compared to:
> > >
> > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> zohar@linux.ibm.com/
> > >
> > > In the original proposal, a new signature version has been introduced (v3)
> > > to allow the possibility of signing the digest of a more flexible data
> > > structure, ima_file_id, which could also include the fsverity file digest.
> > >
> > > While the new signature type would be sufficient to handle fsverity file
> > > digests, the problem is that its format would not be compatible with the
> > > signature format supported by the built-in verification module in fsverity.
> > > The rpm package manager already has an extension to include fsverity
> > > signatures, with the existing format, in the RPM header.
> > >
> > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > the capability of handling it with the existing code, more specifically the
> > > modsig code. It would be sufficient to provide to modsig the correct data
> > > to avoid introducing a new signature format.
> >
> > I think it would be best to get people moved off of the fs-verity built-in
> > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > terrible signature format.  The IMA one is better, though it's unfortunate that
> > IMA still relies on X.509 for keys.
> 
> Note, the only reason that support for fs-verity built-in signatures was added
> to RPM is that people didn't want to use IMA:
> https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> b46a6880824b@gmail.com
> 
> If people are going to use IMA anyway, then there would be no point.

Hi Eric

I thought that the solution I came with could satisfy multiple needs.

For people that don't want to use IMA, they could still continue
to use the existing signature format, and wait for an LSM that
satisfy their needs. They also have the option to migrate to the
new signature format you are defining. But will those people be
willing to switch to something IMA-specific?

For people that use IMA, they could benefit from the effort
of people creating packages with the original fsverity signature.

For people that are skeptical about IMA, they could be interested
in trying the full solution, which would probably be more easily
available if the efforts from both sides converge.

If, as you say, you have concerns about the existing signature
format, wouldn't it be better that you address them from the
fsverity side, so that all users of fsverity can benefit from it?

Currently, fsverity hashes the formatted digest whose format
is FSVerity<digest algo><digest size><digest>. Couldn't IMA
hash the same data as well?

An idea could be to always sign the formatted digest, and have
a selector for the signature format: IMA, PKCS#7 or PGP.

Thanks

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Eric Biggers]

On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Thursday, January 27, 2022 8:40 PM
> > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > I wanted to propose a different approach for handling fsverity digests and
> > > > signatures, compared to:
> > > >
> > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > zohar@linux.ibm.com/
> > > >
> > > > In the original proposal, a new signature version has been introduced (v3)
> > > > to allow the possibility of signing the digest of a more flexible data
> > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > >
> > > > While the new signature type would be sufficient to handle fsverity file
> > > > digests, the problem is that its format would not be compatible with the
> > > > signature format supported by the built-in verification module in fsverity.
> > > > The rpm package manager already has an extension to include fsverity
> > > > signatures, with the existing format, in the RPM header.
> > > >
> > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > the capability of handling it with the existing code, more specifically the
> > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > to avoid introducing a new signature format.
> > >
> > > I think it would be best to get people moved off of the fs-verity built-in
> > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > terrible signature format.  The IMA one is better, though it's unfortunate that
> > > IMA still relies on X.509 for keys.
> > 
> > Note, the only reason that support for fs-verity built-in signatures was added
> > to RPM is that people didn't want to use IMA:
> > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > b46a6880824b@gmail.com
> > 
> > If people are going to use IMA anyway, then there would be no point.
> 
> Hi Eric
> 
> I thought that the solution I came with could satisfy multiple needs.
> 
> For people that don't want to use IMA, they could still continue
> to use the existing signature format, and wait for an LSM that
> satisfy their needs. They also have the option to migrate to the
> new signature format you are defining. But will those people be
> willing to switch to something IMA-specific?
> 
> For people that use IMA, they could benefit from the effort
> of people creating packages with the original fsverity signature.
> 
> For people that are skeptical about IMA, they could be interested
> in trying the full solution, which would probably be more easily
> available if the efforts from both sides converge.
> 
> If, as you say, you have concerns about the existing signature
> format, wouldn't it be better that you address them from the
> fsverity side, so that all users of fsverity can benefit from it?
> 
> Currently, fsverity hashes the formatted digest whose format
> is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> hash the same data as well?
> 
> An idea could be to always sign the formatted digest, and have
> a selector for the signature format: IMA, PKCS#7 or PGP.

Adding support for the new IMA signature format to fsverity_verify_signature()
*might* make sense.  (When I added this code, my understanding was that it was
just verifying signatures the way the kernel usually verifies signatures.  I
don't think I realized there was a more direct, PKCS#7-less way to do it and
that IMA used that way.)  However, it would be better to use this as an
opportunity to move people off of the built-in signatures entirely, either by
switching to a full userspace solution or by switching to IMA.

Part of the problem with IMA is that no one wants to use it because it has
terrible documentation.  It sounds like it's really complicated, and tied to
specific TCG standards and to TPMs.  I think if it was documented better, people
would find it more attractive and wouldn't be trying to avoid it at all costs.

- Eric

RE: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Roberto Sassu]

> From: Eric Biggers [mailto:ebiggers@kernel.org]
> Sent: Friday, January 28, 2022 9:26 PM
> On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > > Sent: Thursday, January 27, 2022 8:40 PM
> > > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > > I wanted to propose a different approach for handling fsverity digests
> and
> > > > > signatures, compared to:
> > > > >
> > > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > > zohar@linux.ibm.com/
> > > > >
> > > > > In the original proposal, a new signature version has been introduced (v3)
> > > > > to allow the possibility of signing the digest of a more flexible data
> > > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > > >
> > > > > While the new signature type would be sufficient to handle fsverity file
> > > > > digests, the problem is that its format would not be compatible with the
> > > > > signature format supported by the built-in verification module in fsverity.
> > > > > The rpm package manager already has an extension to include fsverity
> > > > > signatures, with the existing format, in the RPM header.
> > > > >
> > > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > > the capability of handling it with the existing code, more specifically the
> > > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > > to avoid introducing a new signature format.
> > > >
> > > > I think it would be best to get people moved off of the fs-verity built-in
> > > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > > terrible signature format.  The IMA one is better, though it's unfortunate
> that
> > > > IMA still relies on X.509 for keys.
> > >
> > > Note, the only reason that support for fs-verity built-in signatures was added
> > > to RPM is that people didn't want to use IMA:
> > > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > > b46a6880824b@gmail.com
> > >
> > > If people are going to use IMA anyway, then there would be no point.
> >
> > Hi Eric
> >
> > I thought that the solution I came with could satisfy multiple needs.
> >
> > For people that don't want to use IMA, they could still continue
> > to use the existing signature format, and wait for an LSM that
> > satisfy their needs. They also have the option to migrate to the
> > new signature format you are defining. But will those people be
> > willing to switch to something IMA-specific?
> >
> > For people that use IMA, they could benefit from the effort
> > of people creating packages with the original fsverity signature.
> >
> > For people that are skeptical about IMA, they could be interested
> > in trying the full solution, which would probably be more easily
> > available if the efforts from both sides converge.
> >
> > If, as you say, you have concerns about the existing signature
> > format, wouldn't it be better that you address them from the
> > fsverity side, so that all users of fsverity can benefit from it?
> >
> > Currently, fsverity hashes the formatted digest whose format
> > is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> > hash the same data as well?
> >
> > An idea could be to always sign the formatted digest, and have
> > a selector for the signature format: IMA, PKCS#7 or PGP.
> 
> Adding support for the new IMA signature format to fsverity_verify_signature()
> *might* make sense.  (When I added this code, my understanding was that it
> was
> just verifying signatures the way the kernel usually verifies signatures.  I

Ok. Do we need something more to sign other than the fsverity
formatted digest? If not, this could be the same for any method
we support.

> don't think I realized there was a more direct, PKCS#7-less way to do it and
> that IMA used that way.)  However, it would be better to use this as an
> opportunity to move people off of the built-in signatures entirely, either by
> switching to a full userspace solution or by switching to IMA.

If what we sign remains the same, then we could support multiple
methods and use a selector to let fsverity_verify_signature() know
how it should verify the signature. I don't know what would be a
proper place for the selector.

PKCS#7 seems ok, as it is used for kernel modules. IMA would be
also ok, as it can verify the signature more directly. I would also
be interested in supporting PGP, to avoid the requirement for
Linux distributions to manage a secondary key. I have a small
extension for rpmsign, that I would like to test in the Fedora
infrastructure.

Both the PKCS#7 and the PGP methods don't require additional
support from outside, the functions verify_pkcs7_signature()
and verify_pgp_signature() (proposed, not yet in the upstream
kernel) would be sufficient.

The IMA method instead would require the signature_v2_hdr
structure to be exported to user space, so that rpm could
produce a blob that can be interpreted by the kernel (this
work could also be done by evmctl). Also, IMA should pass
its .ima keyring to fsverity for signature verification, or should
simply get the signature and do the verification internally.

Given that fsverity has already the capability of managing the
signature blob, it would make sense to still keep it. Adding it
in an xattr could be possible, but it would introduce more
constraints (requiring the filesystem to support xattrs). And,
an user of fsverity willing to use the IMA method would have
to look at security.ima.

To summarize: I would prefer a method that relies on an
existing signature verification mechanism (PKCS#7) or that
has an equivalent API and simplify support for Linux distributions
(PGP). If we add the IMA method, available outside IMA, we
need to also add support for user space so that it can produces
the signature in the desired format, and preferably should use
the fsverity way of getting the signature. If the IMA method
would be used by IMA only, then IMA could store the signature
in its xattr and do the verification independently.

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> Part of the problem with IMA is that no one wants to use it because it has
> terrible documentation.  It sounds like it's really complicated, and tied to
> specific TCG standards and to TPMs.  I think if it was documented better, people
> would find it more attractive and wouldn't be trying to avoid it at all costs.
> 
> - Eric

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Stefan Berger]

On 1/31/22 10:12, Roberto Sassu wrote:
>> From: Eric Biggers [mailto:ebiggers@kernel.org]
>> Sent: Friday, January 28, 2022 9:26 PM
>> On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
>>>> From: Eric Biggers [mailto:ebiggers@kernel.org]
>>>> Sent: Thursday, January 27, 2022 8:40 PM
>>>> On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
>>>>> On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
>>>>>> I wanted to propose a different approach for handling fsverity digests
>> and
>>>>>> signatures, compared to:
>>>>>>
>>>>>> https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
>>>> zohar@linux.ibm.com/
>>>>>> In the original proposal, a new signature version has been introduced (v3)
>>>>>> to allow the possibility of signing the digest of a more flexible data
>>>>>> structure, ima_file_id, which could also include the fsverity file digest.
>>>>>>
>>>>>> While the new signature type would be sufficient to handle fsverity file
>>>>>> digests, the problem is that its format would not be compatible with the
>>>>>> signature format supported by the built-in verification module in fsverity.
>>>>>> The rpm package manager already has an extension to include fsverity
>>>>>> signatures, with the existing format, in the RPM header.
>>>>>>
>>>>>> Given that the fsverity signature is in the PKCS#7 format, IMA has already
>>>>>> the capability of handling it with the existing code, more specifically the
>>>>>> modsig code. It would be sufficient to provide to modsig the correct data
>>>>>> to avoid introducing a new signature format.
>>>>> I think it would be best to get people moved off of the fs-verity built-in
>>>>> signatures, rather than further extend the use of it.  PKCS#7 is a pretty
>>>>> terrible signature format.  The IMA one is better, though it's unfortunate
>> that
>>>>> IMA still relies on X.509 for keys.
>>>> Note, the only reason that support for fs-verity built-in signatures was added
>>>> to RPM is that people didn't want to use IMA:
>>>> https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
>>>> b46a6880824b@gmail.com
>>>>
>>>> If people are going to use IMA anyway, then there would be no point.
>>> Hi Eric
>>>
>>> I thought that the solution I came with could satisfy multiple needs.
>>>
>>> For people that don't want to use IMA, they could still continue
>>> to use the existing signature format, and wait for an LSM that
>>> satisfy their needs. They also have the option to migrate to the
>>> new signature format you are defining. But will those people be
>>> willing to switch to something IMA-specific?
>>>
>>> For people that use IMA, they could benefit from the effort
>>> of people creating packages with the original fsverity signature.
>>>
>>> For people that are skeptical about IMA, they could be interested
>>> in trying the full solution, which would probably be more easily
>>> available if the efforts from both sides converge.
>>>
>>> If, as you say, you have concerns about the existing signature
>>> format, wouldn't it be better that you address them from the
>>> fsverity side, so that all users of fsverity can benefit from it?
>>>
>>> Currently, fsverity hashes the formatted digest whose format
>>> is FSVerity<digest algo><digest size><digest>. Couldn't IMA
>>> hash the same data as well?
>>>
>>> An idea could be to always sign the formatted digest, and have
>>> a selector for the signature format: IMA, PKCS#7 or PGP.
>> Adding support for the new IMA signature format to fsverity_verify_signature()
>> *might* make sense.  (When I added this code, my understanding was that it
>> was
>> just verifying signatures the way the kernel usually verifies signatures.  I
> Ok. Do we need something more to sign other than the fsverity
> formatted digest? If not, this could be the same for any method
> we support.
>
>> don't think I realized there was a more direct, PKCS#7-less way to do it and
>> that IMA used that way.)  However, it would be better to use this as an
>> opportunity to move people off of the built-in signatures entirely, either by
>> switching to a full userspace solution or by switching to IMA.
> If what we sign remains the same, then we could support multiple
> methods and use a selector to let fsverity_verify_signature() know
> how it should verify the signature. I don't know what would be a
> proper place for the selector.
>
> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> also ok, as it can verify the signature more directly. I would also
> be interested in supporting PGP, to avoid the requirement for
> Linux distributions to manage a secondary key. I have a small
> extension for rpmsign, that I would like to test in the Fedora
> infrastructure.
>
> Both the PKCS#7 and the PGP methods don't require additional
> support from outside, the functions verify_pkcs7_signature()
> and verify_pgp_signature() (proposed, not yet in the upstream
> kernel) would be sufficient.

FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 
generates a signature of size 817 bytes. If an RPM needs to carry such 
signatures on a per-file basis we are back to the size increase of 
nearly an RSA signature. I would say for packages this is probably too 
much size increase.. and this is what drove the implementation of ECC 
support.

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Eric Biggers]

On Mon, Jan 31, 2022 at 02:29:19PM -0500, Stefan Berger wrote:
> > > don't think I realized there was a more direct, PKCS#7-less way to do it and
> > > that IMA used that way.)  However, it would be better to use this as an
> > > opportunity to move people off of the built-in signatures entirely, either by
> > > switching to a full userspace solution or by switching to IMA.
> > If what we sign remains the same, then we could support multiple
> > methods and use a selector to let fsverity_verify_signature() know
> > how it should verify the signature. I don't know what would be a
> > proper place for the selector.
> > 
> > PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> > also ok, as it can verify the signature more directly. I would also
> > be interested in supporting PGP, to avoid the requirement for
> > Linux distributions to manage a secondary key. I have a small
> > extension for rpmsign, that I would like to test in the Fedora
> > infrastructure.
> > 
> > Both the PKCS#7 and the PGP methods don't require additional
> > support from outside, the functions verify_pkcs7_signature()
> > and verify_pgp_signature() (proposed, not yet in the upstream
> > kernel) would be sufficient.
> 
> FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 generates
> a signature of size 817 bytes. If an RPM needs to carry such signatures on a
> per-file basis we are back to the size increase of nearly an RSA signature.
> I would say for packages this is probably too much size increase.. and this
> is what drove the implementation of ECC support.

I am getting 256 bytes for an ECC signature in PKCS#7 format:

	cd src/fsverity-utils
	make
	openssl ecparam -name prime256v1 -genkey -noout -out key.pem
	openssl req -new -x509 -key key.pem -out cert.pem -days 360
	touch file
	./fsverity sign file file.sig --key=key.pem --cert=cert.pem
	stat -c %s file.sig

Probably you accidentally included the whole certificate in the PKCS#7 message.
That's not required here.

There are definitely problems with PKCS#7, and it does have space overhead.  But
the space overhead is not as bad as you state.

- Eric

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Eric Biggers]

On Mon, Jan 31, 2022 at 03:12:42PM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Friday, January 28, 2022 9:26 PM
> > On Fri, Jan 28, 2022 at 09:05:01AM +0000, Roberto Sassu wrote:
> > > > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > > > Sent: Thursday, January 27, 2022 8:40 PM
> > > > On Thu, Jan 27, 2022 at 11:35:12AM -0800, Eric Biggers wrote:
> > > > > On Thu, Jan 27, 2022 at 07:46:09PM +0100, Roberto Sassu wrote:
> > > > > > I wanted to propose a different approach for handling fsverity digests
> > and
> > > > > > signatures, compared to:
> > > > > >
> > > > > > https://lore.kernel.org/linux-integrity/20220126000658.138345-1-
> > > > zohar@linux.ibm.com/
> > > > > >
> > > > > > In the original proposal, a new signature version has been introduced (v3)
> > > > > > to allow the possibility of signing the digest of a more flexible data
> > > > > > structure, ima_file_id, which could also include the fsverity file digest.
> > > > > >
> > > > > > While the new signature type would be sufficient to handle fsverity file
> > > > > > digests, the problem is that its format would not be compatible with the
> > > > > > signature format supported by the built-in verification module in fsverity.
> > > > > > The rpm package manager already has an extension to include fsverity
> > > > > > signatures, with the existing format, in the RPM header.
> > > > > >
> > > > > > Given that the fsverity signature is in the PKCS#7 format, IMA has already
> > > > > > the capability of handling it with the existing code, more specifically the
> > > > > > modsig code. It would be sufficient to provide to modsig the correct data
> > > > > > to avoid introducing a new signature format.
> > > > >
> > > > > I think it would be best to get people moved off of the fs-verity built-in
> > > > > signatures, rather than further extend the use of it.  PKCS#7 is a pretty
> > > > > terrible signature format.  The IMA one is better, though it's unfortunate
> > that
> > > > > IMA still relies on X.509 for keys.
> > > >
> > > > Note, the only reason that support for fs-verity built-in signatures was added
> > > > to RPM is that people didn't want to use IMA:
> > > > https://lore.kernel.org/linux-fscrypt/b49b4367-51e7-f62a-6209-
> > > > b46a6880824b@gmail.com
> > > >
> > > > If people are going to use IMA anyway, then there would be no point.
> > >
> > > Hi Eric
> > >
> > > I thought that the solution I came with could satisfy multiple needs.
> > >
> > > For people that don't want to use IMA, they could still continue
> > > to use the existing signature format, and wait for an LSM that
> > > satisfy their needs. They also have the option to migrate to the
> > > new signature format you are defining. But will those people be
> > > willing to switch to something IMA-specific?
> > >
> > > For people that use IMA, they could benefit from the effort
> > > of people creating packages with the original fsverity signature.
> > >
> > > For people that are skeptical about IMA, they could be interested
> > > in trying the full solution, which would probably be more easily
> > > available if the efforts from both sides converge.
> > >
> > > If, as you say, you have concerns about the existing signature
> > > format, wouldn't it be better that you address them from the
> > > fsverity side, so that all users of fsverity can benefit from it?
> > >
> > > Currently, fsverity hashes the formatted digest whose format
> > > is FSVerity<digest algo><digest size><digest>. Couldn't IMA
> > > hash the same data as well?
> > >
> > > An idea could be to always sign the formatted digest, and have
> > > a selector for the signature format: IMA, PKCS#7 or PGP.
> > 
> > Adding support for the new IMA signature format to fsverity_verify_signature()
> > *might* make sense.  (When I added this code, my understanding was that it
> > was
> > just verifying signatures the way the kernel usually verifies signatures.  I
> 
> Ok. Do we need something more to sign other than the fsverity
> formatted digest? If not, this could be the same for any method
> we support.
> 
> > don't think I realized there was a more direct, PKCS#7-less way to do it and
> > that IMA used that way.)  However, it would be better to use this as an
> > opportunity to move people off of the built-in signatures entirely, either by
> > switching to a full userspace solution or by switching to IMA.
> 
> If what we sign remains the same, then we could support multiple
> methods and use a selector to let fsverity_verify_signature() know
> how it should verify the signature. I don't know what would be a
> proper place for the selector.
> 
> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
> also ok, as it can verify the signature more directly. I would also
> be interested in supporting PGP, to avoid the requirement for
> Linux distributions to manage a secondary key. I have a small
> extension for rpmsign, that I would like to test in the Fedora
> infrastructure.
> 
> Both the PKCS#7 and the PGP methods don't require additional
> support from outside, the functions verify_pkcs7_signature()
> and verify_pgp_signature() (proposed, not yet in the upstream
> kernel) would be sufficient.
> 
> The IMA method instead would require the signature_v2_hdr
> structure to be exported to user space, so that rpm could
> produce a blob that can be interpreted by the kernel (this
> work could also be done by evmctl). Also, IMA should pass
> its .ima keyring to fsverity for signature verification, or should
> simply get the signature and do the verification internally.
> 
> Given that fsverity has already the capability of managing the
> signature blob, it would make sense to still keep it. Adding it
> in an xattr could be possible, but it would introduce more
> constraints (requiring the filesystem to support xattrs). And,
> an user of fsverity willing to use the IMA method would have
> to look at security.ima.
> 
> To summarize: I would prefer a method that relies on an
> existing signature verification mechanism (PKCS#7) or that
> has an equivalent API and simplify support for Linux distributions
> (PGP). If we add the IMA method, available outside IMA, we
> need to also add support for user space so that it can produces
> the signature in the desired format, and preferably should use
> the fsverity way of getting the signature. If the IMA method
> would be used by IMA only, then IMA could store the signature
> in its xattr and do the verification independently.
> 
> Roberto
> 

I think you are conflating the signatures themselves from where they are stored.
The fs-verity built-in signatures feature could be extended to support the same
signatures as IMA, while still storing the signature in the same way the
fs-verity built-in signatures are currently stored (which doesn't use xattrs).

But as I said, I don't think it makes sense to continue building on the
fs-verity built-in signatures feature, as opposed to guiding users towards a
full userspace solution or to IMA instead.

- Eric

Re: [RFC][PATCH v3a 00/11] ima: support fs-verity digests and signatures (alternative)

[Stefan Berger]

On 1/31/22 15:24, Eric Biggers wrote:
> On Mon, Jan 31, 2022 at 02:29:19PM -0500, Stefan Berger wrote:
>>>> don't think I realized there was a more direct, PKCS#7-less way to do it and
>>>> that IMA used that way.)  However, it would be better to use this as an
>>>> opportunity to move people off of the built-in signatures entirely, either by
>>>> switching to a full userspace solution or by switching to IMA.
>>> If what we sign remains the same, then we could support multiple
>>> methods and use a selector to let fsverity_verify_signature() know
>>> how it should verify the signature. I don't know what would be a
>>> proper place for the selector.
>>>
>>> PKCS#7 seems ok, as it is used for kernel modules. IMA would be
>>> also ok, as it can verify the signature more directly. I would also
>>> be interested in supporting PGP, to avoid the requirement for
>>> Linux distributions to manage a secondary key. I have a small
>>> extension for rpmsign, that I would like to test in the Fedora
>>> infrastructure.
>>>
>>> Both the PKCS#7 and the PGP methods don't require additional
>>> support from outside, the functions verify_pkcs7_signature()
>>> and verify_pgp_signature() (proposed, not yet in the upstream
>>> kernel) would be sufficient.
>> FYI: An empty file signed with pkcs7 and an ecc key for NIST p256 generates
>> a signature of size 817 bytes. If an RPM needs to carry such signatures on a
>> per-file basis we are back to the size increase of nearly an RSA signature.
>> I would say for packages this is probably too much size increase.. and this
>> is what drove the implementation of ECC support.
> I am getting 256 bytes for an ECC signature in PKCS#7 format:
>
> 	cd src/fsverity-utils
> 	make
> 	openssl ecparam -name prime256v1 -genkey -noout -out key.pem
> 	openssl req -new -x509 -key key.pem -out cert.pem -days 360
> 	touch file
> 	./fsverity sign file file.sig --key=key.pem --cert=cert.pem
> 	stat -c %s file.sig
>
> Probably you accidentally included the whole certificate in the PKCS#7 message.
> That's not required here.
>
> There are definitely problems with PKCS#7, and it does have space overhead.  But
> the space overhead is not as bad as you state.

You are right. I used openssl cms without -nocerts and -noattr 
(unintentionately). Though 256 bytes is RSA 2048 signature size again. 
ECDSA with NIST p256 key is around 70 bytes.


>
> - Eric