[RFC][PATCH v3 00/10] ima: Integrate with Integrity Digest Cache

[RFC][PATCH v3 00/10] ima: Integrate with Integrity Digest Cache

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

One of the IMA shortcomings over the years has been the availability of
reference digest values for appraisal. Recently, the situation improved
and some Linux distributions are including file signatures, such as
Fedora 39.

The Integrity Digest Cache takes a different approach. Instead of requiring
Linux distributions to include file signatures in their packages, it parses
the digests from signed RPM package headers and exposes an API for
integrity providers to query a digest.

That enables Linux distributions to immediately gain the ability to do
integrity checks with the existing packages, lowering the burden for
software vendors.

In addition, integrating IMA with the Integrity Digest Cache has even more
benefits.

First, it allows generating a new-style masurement list including the RPM
package headers and the unknown files, which improves system performance
due to the lower usage of the TPM. The cost is the less accuracy of the
information reported, which might not suitable for everyone.

Second, performance improve for appraisal too. It has been found that
verifying the signatures of only the RPM package headers and doing a digest
lookup is much less computationally expensive than verifying individual
file signatures.

In the future, if RPM and other package formats include fsverity digests,
this would further improve the performance, due to verifying only the
portion of the file read.

For reference, a preliminary performance evaluation has been published
here:

https://lore.kernel.org/linux-integrity/20240905150543.3766895-15-roberto.sassu@huaweicloud.com/

Third, it makes a PCR predictable and suitable for TPM key sealing
policies.

Finally, it allows IMA to maintain a predictable PCR and to perform
appraisal from the very beginning of the boot, in the initial ram disk
(excluding auto-generated files).

Integration of IMA with the digest_cache LSM is straightforward.

Patch 1 lets IMA know when the digest_cache LSM is reading a digest list,
to populate a digest cache.

Patch 2 allows nested IMA verification of digest lists read by the
digest_cache LSM.

Patch 3 allows the usage of digest caches with the IMA policy.

Patch 4 introduces new boot-time built-in policies, to use digest caches
from the very beginning (it allows measurement/appraisal from the initial
ram disk).

Patch 5 modifies existing boot-time built-in policies if the digest_cache
LSM-specific policies have been selected at boot.

Patch 6 obtains a digest cache for a given file, stores it in the inode
integrity metadata, and notifies if the digest cache changed since last
file access.

Patches 7-8 store and load the integrity state of the digest list the
digest cache was populated from, to restrict the digest cache usage in case
an IMA action was not performed on the digest list.

Patches 9-10 enable the usage of digest caches respectively for measurement
and appraisal, at the condition that it is authorized with the IMA policy
and that the digest list itself was measured and appraised too.

Open points:
- Mimi prefers to extend flags in ima_iint_cache, rather than passing the
  parameter down to process_measurement() - will do in a next version
- Prefetching of digest lists should not be done if there is no
  measurement rule (not relevant for appraisal)


This patch set applies on top of:

https://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git/log/?h=next-integrity

with commit fa8a4ce432e8 ("ima: fix buffer overrun in
ima_eventdigest_init_common").

and on top of the patch set at the URL:

https://lore.kernel.org/linux-integrity/20240905150543.3766895-1-roberto.sassu@huaweicloud.com/

Changelog

v2:
- Rename digest_cache LSM to Integrity Digest Cache (suggested by Paul
  Moore)
- Add digest_cache member to ima_iint_cache structure
- Nest mutex only in process_measurement()
- Check IMA_DIGEST_CACHE_APPRAISE_DATA earlier in
  ima_appraise_measurement()
- Introduce ima_digest_cache_get_check() to detect changes, instead of
  using the notifier mechanism (removed from the Integrity Digest Cache)
- Rename ima_digest_cache_store_allowed_usage() to
  ima_digest_cache_store_verified_usage()
- Rename ima_digest_cache_update_allowed_usage() to
  ima_digest_cache_load_verified_usage(), but do the AND outside the
  function
- Check allowed IMA hooks with ima_digest_cache_func_allowed() also at
  run-time (when a policy is evaluated)

v1:
- Change digest_cache= policy keyword value from 'content' to 'data'
  (suggested by Mimi)
- Move digest_cache LSM integration code to ima_digest_cache.c (suggested
  by Mimi)
- Don't store digest cache pointer in integrity metadata
- Rename 'digest_cache_mask' parameter of ima_get_action() and
  ima_match_policy() to 'digest_cache_usage'
- Rename 'digest_cache_mask' parameter of ima_store_measurement() and
  ima_appraise_measurement() to 'allowed_usage'
- Try digest cache method as first in ima_appraise_measurement() (suggested
  by Mimi)
- Introduce ima_digest_cache_change() to be called on digest cache reset
- Subscribe to digest cache events
- Add forgotten modification in ima_iint_lockdep_annotate() (reported by
  Mimi)
- Replace 'digest_cache_mask' member of the ima_rule_entry structure with
  'digest_cache_usage' (suggested by Mimi)
- Split patch introducing digest_cache LSM-specific boot-time built-in
  policies and modifying existing rules
- Add digest_cache LSM-specific boot-time built-in policies if the
  digest_cache LSM is enabled in the kernel configuration
- Rename IMA_DIGEST_CACHE_MEASURE_CONTENT and
  IMA_DIGEST_CACHE_APPRAISE_CONTENT to IMA_DIGEST_CACHE_MEASURE_DATA and
  IMA_DIGEST_CACHE_APPRAISE_DATA

Roberto Sassu (10):
  ima: Introduce hook DIGEST_LIST_CHECK
  ima: Nest iint mutex for DIGEST_LIST_CHECK hook
  ima: Add digest_cache policy keyword
  ima: Add digest_cache_measure/appraise boot-time built-in policies
  ima: Modify existing boot-time built-in policies with digest cache
    policies
  ima: Retrieve digest cache and check if changed
  ima: Store verified usage in digest cache based on integrity metadata
    flags
  ima: Load verified usage from digest cache found from query
  ima: Use digest caches for measurement
  ima: Use digest caches for appraisal

 Documentation/ABI/testing/ima_policy          |   6 +-
 .../admin-guide/kernel-parameters.txt         |  15 +-
 security/integrity/ima/Kconfig                |  10 ++
 security/integrity/ima/Makefile               |   1 +
 security/integrity/ima/ima.h                  |  21 ++-
 security/integrity/ima/ima_api.c              |  21 ++-
 security/integrity/ima/ima_appraise.c         |  33 +++--
 security/integrity/ima/ima_digest_cache.c     | 121 ++++++++++++++++
 security/integrity/ima/ima_digest_cache.h     |  38 +++++
 security/integrity/ima/ima_iint.c             |   4 +
 security/integrity/ima/ima_main.c             |  34 +++--
 security/integrity/ima/ima_policy.c           | 133 +++++++++++++++++-
 12 files changed, 409 insertions(+), 28 deletions(-)
 create mode 100644 security/integrity/ima/ima_digest_cache.c
 create mode 100644 security/integrity/ima/ima_digest_cache.h

-- 
2.34.1

[RFC][PATCH v3 01/10] ima: Introduce hook DIGEST_LIST_CHECK

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce a new hook to check the integrity of digest lists.

The new hook is invoked during a kernel read with file type
READING_DIGEST LIST, which is done by the Integrity Digest Cache when it is
populating a digest cache with a digest list.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/ABI/testing/ima_policy | 1 +
 security/integrity/ima/ima.h         | 1 +
 security/integrity/ima/ima_main.c    | 3 ++-
 security/integrity/ima/ima_policy.c  | 3 +++
 4 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index c2385183826c..22237fec5532 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -36,6 +36,7 @@ Description:
 				[KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK]
 				[KEXEC_CMDLINE] [KEY_CHECK] [CRITICAL_DATA]
 				[SETXATTR_CHECK][MMAP_CHECK_REQPROT]
+				[DIGEST_LIST_CHECK]
 			mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
 			       [[^]MAY_EXEC]
 			fsmagic:= hex value
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ad5c95cf22ac..9d41d6b1cce2 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -317,6 +317,7 @@ static inline unsigned int ima_hash_key(u8 *digest)
 	hook(KEY_CHECK, key)				\
 	hook(CRITICAL_DATA, critical_data)		\
 	hook(SETXATTR_CHECK, setxattr_check)		\
+	hook(DIGEST_LIST_CHECK, digest_list_check)	\
 	hook(MAX_CHECK, none)
 
 #define __ima_hook_enumify(ENUM, str)	ENUM,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 646d900828e0..cff8b5a12512 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -798,7 +798,8 @@ const int read_idmap[READING_MAX_ID] = {
 	[READING_MODULE] = MODULE_CHECK,
 	[READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
 	[READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
-	[READING_POLICY] = POLICY_CHECK
+	[READING_POLICY] = POLICY_CHECK,
+	[READING_DIGEST_LIST] = DIGEST_LIST_CHECK,
 };
 
 /**
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 09da8e639239..047d50c2eb57 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -1290,6 +1290,7 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 	case MODULE_CHECK:
 	case KEXEC_KERNEL_CHECK:
 	case KEXEC_INITRAMFS_CHECK:
+	case DIGEST_LIST_CHECK:
 		if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
 				     IMA_UID | IMA_FOWNER | IMA_FSUUID |
 				     IMA_INMASK | IMA_EUID | IMA_PCR |
@@ -1533,6 +1534,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->func = CRITICAL_DATA;
 			else if (strcmp(args[0].from, "SETXATTR_CHECK") == 0)
 				entry->func = SETXATTR_CHECK;
+			else if (strcmp(args[0].from, "DIGEST_LIST_CHECK") == 0)
+				entry->func = DIGEST_LIST_CHECK;
 			else
 				result = -EINVAL;
 			if (!result)
-- 
2.34.1

[RFC][PATCH v3 02/10] ima: Nest iint mutex for DIGEST_LIST_CHECK hook

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Invoking digest_cache_get() inside the iint->mutex critical region can
cause deadlocks due to the fact that IMA can be recursively invoked for
reading the digest list. The deadlock would occur if the Integrity Digest
Cache attempts to read the same inode that is already locked by IMA.

However, since the Integrity Digest Cache makes sure that the above
situation never happens, as it checks the inodes, it is safe to call
digest_cache_get() inside the critical region and nest the iint->mutex
when the DIGEST_LIST_CHECK hook is executed.

Change the mutex_lock() call for the iint mutex into mutex_lock_nested(),
and add a subclass that is 0 if the IMA hook executed is not
DIGEST_LIST_CHECK, and 1 if it is.

Specifying the subclass does not interfere with the class annotation in
ima_iint_lockdep_annotate(), to make IMA work with overlayfs.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index cff8b5a12512..06ee99bd7886 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -266,7 +266,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (!action)
 		goto out;
 
-	mutex_lock(&iint->mutex);
+	mutex_lock_nested(&iint->mutex, func == DIGEST_LIST_CHECK);
 
 	if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags))
 		/* reset appraisal flags if ima_inode_post_setattr was called */
-- 
2.34.1

[RFC][PATCH v3 03/10] ima: Add digest_cache policy keyword

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Add the 'digest_cache=' policy keyword, to enable the usage of digest
caches for specific IMA actions and purpose.

At the moment, it accepts only 'data' as value, as digest caches can be
used only for measurement and appraisal of file data. In the future, it
might be possible to use them for file metadata too.

The 'digest_cache=' keyword can be specified for the subset of IMA hooks
listed in ima_digest_cache_func_allowed(). In case the function is not
specified in the policy, the filtering is done when the policy is
evaluated.

POLICY_CHECK has been excluded for measurement, because policy changes must
be visible in the IMA measurement list. For appraisal, instead, it might be
useful to load custom policies in the initial ram disk (no security.ima
xattr).

Add the digest_cache_usage member to the ima_rule_entry structure, and set
the flag IMA_DIGEST_CACHE_MEASURE_DATA if 'digest_cache=data' was specified
for a measure rule, IMA_DIGEST_CACHE_APPRAISE_DATA for an appraise rule.

Propagate the usage down to ima_match_policy() and ima_get_action(), so
that process_measurement() can make the final decision on whether or not
digest caches should be used to measure/appraise the file being evaluated.

Since using digest caches changes the meaning of the IMA measurement list,
which will include only digest lists and unknown files, enforce specifying
'pcr=' with a non-standard value, when 'digest_cache=data' is specified in
a measure rule.

This removes the ambiguity on the meaning of the IMA measurement list.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/ABI/testing/ima_policy  |  5 +-
 security/integrity/ima/ima.h          | 10 +++-
 security/integrity/ima/ima_api.c      |  6 +-
 security/integrity/ima/ima_appraise.c |  2 +-
 security/integrity/ima/ima_main.c     |  8 +--
 security/integrity/ima/ima_policy.c   | 81 ++++++++++++++++++++++++++-
 6 files changed, 100 insertions(+), 12 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 22237fec5532..887ac79f66eb 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -29,7 +29,7 @@ Description:
 				 [obj_user=] [obj_role=] [obj_type=]]
 			option:	[digest_type=] [template=] [permit_directio]
 				[appraise_type=] [appraise_flag=]
-				[appraise_algos=] [keyrings=]
+				[appraise_algos=] [keyrings=] [digest_cache=]
 		  base:
 			func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
@@ -77,6 +77,9 @@ Description:
 			For example, "sha256,sha512" to only accept to appraise
 			files where the security.ima xattr was hashed with one
 			of these two algorithms.
+			digest_cache:= [data]
+			"data" means that the digest cache is used only
+			for file data measurement and/or appraisal.
 
 		  default policy:
 			# PROC_SUPER_MAGIC
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 9d41d6b1cce2..736fe014afbc 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -44,6 +44,10 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
 #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
 
+/* Digest cache usage flags. */
+#define IMA_DIGEST_CACHE_MEASURE_DATA	0x0000000000000001
+#define IMA_DIGEST_CACHE_APPRAISE_DATA	0x0000000000000002
+
 /* current content of the policy */
 extern int ima_policy_flag;
 
@@ -374,7 +378,8 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode *inode,
 		   const struct cred *cred, u32 secid, int mask,
 		   enum ima_hooks func, int *pcr,
 		   struct ima_template_desc **template_desc,
-		   const char *func_data, unsigned int *allowed_algos);
+		   const char *func_data, unsigned int *allowed_algos,
+		   u64 *digest_cache_usage);
 int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
 int ima_collect_measurement(struct ima_iint_cache *iint, struct file *file,
 			    void *buf, loff_t size, enum hash_algo algo,
@@ -405,7 +410,8 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode *inode,
 		     const struct cred *cred, u32 secid, enum ima_hooks func,
 		     int mask, int flags, int *pcr,
 		     struct ima_template_desc **template_desc,
-		     const char *func_data, unsigned int *allowed_algos);
+		     const char *func_data, unsigned int *allowed_algos,
+		     u64 *digest_cache_usage);
 void ima_init_policy(void);
 void ima_update_policy(void);
 void ima_update_policy_flags(void);
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 984e861f6e33..b44cf7d9fbcb 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -173,6 +173,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename,
  * @template_desc: pointer filled in if matched measure policy sets template=
  * @func_data: func specific data, may be NULL
  * @allowed_algos: allowlist of hash algorithms for the IMA xattr
+ * @digest_cache_usage: Actions and purpose for which digest cache is allowed
  *
  * The policy is defined in terms of keypairs:
  *		subj=, obj=, type=, func=, mask=, fsmagic=
@@ -190,7 +191,8 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode *inode,
 		   const struct cred *cred, u32 secid, int mask,
 		   enum ima_hooks func, int *pcr,
 		   struct ima_template_desc **template_desc,
-		   const char *func_data, unsigned int *allowed_algos)
+		   const char *func_data, unsigned int *allowed_algos,
+		   u64 *digest_cache_usage)
 {
 	int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE | IMA_HASH;
 
@@ -198,7 +200,7 @@ int ima_get_action(struct mnt_idmap *idmap, struct inode *inode,
 
 	return ima_match_policy(idmap, inode, cred, secid, func, mask,
 				flags, pcr, template_desc, func_data,
-				allowed_algos);
+				allowed_algos, digest_cache_usage);
 }
 
 static bool ima_get_verity_digest(struct ima_iint_cache *iint,
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 656c709b974f..d479cf58d859 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -81,7 +81,7 @@ int ima_must_appraise(struct mnt_idmap *idmap, struct inode *inode,
 	security_current_getsecid_subj(&secid);
 	return ima_match_policy(idmap, inode, current_cred(), secid,
 				func, mask, IMA_APPRAISE | IMA_HASH, NULL,
-				NULL, NULL, NULL);
+				NULL, NULL, NULL, NULL);
 }
 
 static int ima_fix_xattr(struct dentry *dentry, struct ima_iint_cache *iint)
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 06ee99bd7886..ebf233664194 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -234,7 +234,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	 */
 	action = ima_get_action(file_mnt_idmap(file), inode, cred, secid,
 				mask, func, &pcr, &template_desc, NULL,
-				&allowed_algos);
+				&allowed_algos, NULL);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK ||
 			    func == MMAP_CHECK_REQPROT) &&
 			   (ima_policy_flag & IMA_MEASURE));
@@ -502,11 +502,11 @@ static int ima_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
 	inode = file_inode(vma->vm_file);
 	action = ima_get_action(file_mnt_idmap(vma->vm_file), inode,
 				current_cred(), secid, MAY_EXEC, MMAP_CHECK,
-				&pcr, &template, NULL, NULL);
+				&pcr, &template, NULL, NULL, NULL);
 	action |= ima_get_action(file_mnt_idmap(vma->vm_file), inode,
 				 current_cred(), secid, MAY_EXEC,
 				 MMAP_CHECK_REQPROT, &pcr, &template, NULL,
-				 NULL);
+				 NULL, NULL);
 
 	/* Is the mmap'ed file in policy? */
 	if (!(action & (IMA_MEASURE | IMA_APPRAISE_SUBMASK)))
@@ -994,7 +994,7 @@ int process_buffer_measurement(struct mnt_idmap *idmap,
 		security_current_getsecid_subj(&secid);
 		action = ima_get_action(idmap, inode, current_cred(),
 					secid, 0, func, &pcr, &template,
-					func_data, NULL);
+					func_data, NULL, NULL);
 		if (!(action & IMA_MEASURE) && !digest)
 			return -ENOENT;
 	}
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 047d50c2eb57..eb3bfe01782d 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -122,6 +122,7 @@ struct ima_rule_entry {
 	struct ima_rule_opt_list *keyrings; /* Measure keys added to these keyrings */
 	struct ima_rule_opt_list *label; /* Measure data grouped under this label */
 	struct ima_template_desc *template;
+	u64 digest_cache_usage;	/* Actions and purpose for which digest cache is allowed */
 };
 
 /*
@@ -714,6 +715,27 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
 	}
 }
 
+/* Filter IMA hooks that can use digest caches. */
+static bool ima_digest_cache_func_allowed(enum ima_hooks func)
+{
+	switch (func) {
+	case NONE:
+	case FILE_CHECK:
+	case MMAP_CHECK:
+	case MMAP_CHECK_REQPROT:
+	case BPRM_CHECK:
+	case CREDS_CHECK:
+	case FIRMWARE_CHECK:
+	case POLICY_CHECK:
+	case MODULE_CHECK:
+	case KEXEC_KERNEL_CHECK:
+	case KEXEC_INITRAMFS_CHECK:
+		return true;
+	default:
+		return false;
+	}
+}
+
 /**
  * ima_match_policy - decision based on LSM and other conditions
  * @idmap: idmap of the mount the inode was found from
@@ -728,6 +750,7 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func)
  * @template_desc: the template that should be used for this rule
  * @func_data: func specific data, may be NULL
  * @allowed_algos: allowlist of hash algorithms for the IMA xattr
+ * @digest_cache_usage: Actions and purpose for which digest cache is allowed
  *
  * Measure decision based on func/mask/fsmagic and LSM(subj/obj/type)
  * conditions.
@@ -740,7 +763,8 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode *inode,
 		     const struct cred *cred, u32 secid, enum ima_hooks func,
 		     int mask, int flags, int *pcr,
 		     struct ima_template_desc **template_desc,
-		     const char *func_data, unsigned int *allowed_algos)
+		     const char *func_data, unsigned int *allowed_algos,
+		     u64 *digest_cache_usage)
 {
 	struct ima_rule_entry *entry;
 	int action = 0, actmask = flags | (flags << 1);
@@ -785,6 +809,22 @@ int ima_match_policy(struct mnt_idmap *idmap, struct inode *inode,
 		if (template_desc && entry->template)
 			*template_desc = entry->template;
 
+		/*
+		 * Since we allow IMA policy rules without func=, check if the
+		 * current IMA hook is allowed and, if not, disregard the digest
+		 * cache usage from the policy.
+		 *
+		 * In addition, don't allow digest caches to be used for IMA
+		 * policy measurements, so that policies always appear in the
+		 * measurement list.
+		 */
+		if (digest_cache_usage && ima_digest_cache_func_allowed(func)) {
+			*digest_cache_usage |= entry->digest_cache_usage;
+
+			if (func == POLICY_CHECK)
+				*digest_cache_usage &= ~IMA_DIGEST_CACHE_MEASURE_DATA;
+		}
+
 		if (!actmask)
 			break;
 	}
@@ -1075,7 +1115,7 @@ enum policy_opt {
 	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
+	Opt_label, Opt_digest_cache, Opt_err
 };
 
 static const match_table_t policy_tokens = {
@@ -1124,6 +1164,7 @@ static const match_table_t policy_tokens = {
 	{Opt_template, "template=%s"},
 	{Opt_keyrings, "keyrings=%s"},
 	{Opt_label, "label=%s"},
+	{Opt_digest_cache, "digest_cache=%s"},
 	{Opt_err, NULL}
 };
 
@@ -1248,6 +1289,19 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
 	if (entry->action != MEASURE && entry->flags & IMA_PCR)
 		return false;
 
+	/* New-style measurements with digest cache cannot be on default PCR. */
+	if (entry->action == MEASURE &&
+	    (entry->digest_cache_usage & IMA_DIGEST_CACHE_MEASURE_DATA)) {
+		if (!(entry->flags & IMA_PCR) ||
+		    entry->pcr == CONFIG_IMA_MEASURE_PCR_IDX)
+			return false;
+	}
+
+	/* Digest caches can be used only for a subset of the IMA hooks. */
+	if (entry->digest_cache_usage &&
+	    !ima_digest_cache_func_allowed(entry->func))
+		return false;
+
 	if (entry->action != APPRAISE &&
 	    entry->flags & (IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED |
 			    IMA_CHECK_BLACKLIST | IMA_VALIDATE_ALGOS))
@@ -1884,6 +1938,26 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 						 &(template_desc->num_fields));
 			entry->template = template_desc;
 			break;
+		case Opt_digest_cache:
+			ima_log_string(ab, "digest_cache", args[0].from);
+
+			result = -EINVAL;
+
+			if (!strcmp(args[0].from, "data")) {
+				switch (entry->action) {
+				case MEASURE:
+					entry->digest_cache_usage |= IMA_DIGEST_CACHE_MEASURE_DATA;
+					result = 0;
+					break;
+				case APPRAISE:
+					entry->digest_cache_usage |= IMA_DIGEST_CACHE_APPRAISE_DATA;
+					result = 0;
+					break;
+				default:
+					break;
+				}
+			}
+			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
 			result = -EINVAL;
@@ -2274,6 +2348,9 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, "digest_type=verity ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
+	if ((entry->digest_cache_usage & IMA_DIGEST_CACHE_MEASURE_DATA) ||
+	    (entry->digest_cache_usage & IMA_DIGEST_CACHE_APPRAISE_DATA))
+		seq_puts(m, "digest_cache=data ");
 	rcu_read_unlock();
 	seq_puts(m, "\n");
 	return 0;
-- 
2.34.1

[RFC][PATCH v3 04/10] ima: Add digest_cache_measure/appraise boot-time built-in policies

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Specify the 'digest_cache_measure' boot-time policy with 'ima_policy=' in
the kernel command line to add the following rule at the beginning of the
IMA policy, before other rules:

measure func=DIGEST_LIST_CHECK pcr=12

which will measure digest lists into PCR 12 (or the value in
CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX).

Specify 'digest_cache_appraise' to add the following rule at the beginning,
before other rules:

appraise func=DIGEST_LIST_CHECK appraise_type=imasig|modsig

which will appraise digest lists with IMA signatures or module-style
appended signatures.

Adding those rule at the beginning rather than at the end is necessary to
ensure that digest lists are measured and appraised in the initial ram
disk, which would be otherwise prevented by the dont_ rules.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 .../admin-guide/kernel-parameters.txt         | 10 +++++-
 security/integrity/ima/Kconfig                | 10 ++++++
 security/integrity/ima/ima_policy.c           | 35 +++++++++++++++++++
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 09126bb8cc9f..afaaf125a237 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2077,7 +2077,8 @@
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
 			Format: "tcb | appraise_tcb | secure_boot |
-				 fail_securely | critical_data"
+				 fail_securely | critical_data |
+				 digest_cache_measure | digest_cache_appraise"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read
@@ -2099,6 +2100,13 @@
 			The "critical_data" policy measures kernel integrity
 			critical data.
 
+			The "digest_cache_measure" policy measures digest lists
+			into PCR 12 (can be changed with kernel config).
+
+			The "digest_cache_appraise" policy appraises digest
+			lists with IMA signatures or module-style appended
+			signatures.
+
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 475c32615006..d2d79185f714 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -321,4 +321,14 @@ config IMA_DISABLE_HTABLE
 	help
 	   This option disables htable to allow measurement of duplicate records.
 
+config IMA_DIGEST_CACHE_MEASURE_PCR_IDX
+	int
+	range 8 14
+	default 12
+	help
+	  This option determines the TPM PCR register index that IMA uses to
+	  maintain the integrity aggregate of the measurement list, when the
+	  Integrity Digest Cache is used (different measurement style).
+	  If unsure, use the default 12.
+
 endif
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index eb3bfe01782d..c2bf58010b89 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -254,6 +254,21 @@ static struct ima_rule_entry critical_data_rules[] __ro_after_init = {
 	{.action = MEASURE, .func = CRITICAL_DATA, .flags = IMA_FUNC},
 };
 
+static struct ima_rule_entry measure_digest_cache_rule __ro_after_init = {
+#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
+	.action = MEASURE, .func = DIGEST_LIST_CHECK,
+	.pcr = CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX,
+	.flags = IMA_FUNC | IMA_PCR
+#endif
+};
+
+static struct ima_rule_entry appraise_digest_cache_rule __ro_after_init = {
+#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
+	.action = APPRAISE, .func = DIGEST_LIST_CHECK,
+	.flags = IMA_FUNC | IMA_DIGSIG_REQUIRED | IMA_MODSIG_ALLOWED,
+#endif
+};
+
 /* An array of architecture specific rules */
 static struct ima_rule_entry *arch_policy_entry __ro_after_init;
 
@@ -278,6 +293,8 @@ static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
 static bool ima_use_critical_data __initdata;
 static bool ima_fail_unverifiable_sigs __ro_after_init;
+static bool ima_digest_cache_measure __ro_after_init;
+static bool ima_digest_cache_appraise __ro_after_init;
 static int __init policy_setup(char *str)
 {
 	char *p;
@@ -295,6 +312,10 @@ static int __init policy_setup(char *str)
 			ima_use_critical_data = true;
 		else if (strcmp(p, "fail_securely") == 0)
 			ima_fail_unverifiable_sigs = true;
+		else if (strcmp(p, "digest_cache_measure") == 0)
+			ima_digest_cache_measure = true;
+		else if (strcmp(p, "digest_cache_appraise") == 0)
+			ima_digest_cache_appraise = true;
 		else
 			pr_err("policy \"%s\" not found", p);
 	}
@@ -983,6 +1004,20 @@ void __init ima_init_policy(void)
 {
 	int build_appraise_entries, arch_entries;
 
+	/*
+	 * We need to load digest cache rules at the beginning, to avoid dont_
+	 * rules causing ours to not be reached.
+	 */
+	if (IS_ENABLED(CONFIG_INTEGRITY_DIGEST_CACHE)) {
+		if (ima_digest_cache_measure)
+			add_rules(&measure_digest_cache_rule, 1,
+				  IMA_DEFAULT_POLICY);
+
+		if (ima_digest_cache_appraise)
+			add_rules(&appraise_digest_cache_rule, 1,
+				  IMA_DEFAULT_POLICY);
+	}
+
 	/* if !ima_policy, we load NO default rules */
 	if (ima_policy)
 		add_rules(dont_measure_rules, ARRAY_SIZE(dont_measure_rules),
-- 
2.34.1

[RFC][PATCH v3 05/10] ima: Modify existing boot-time built-in policies with digest cache policies

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Setting the boot-time built-in policies 'digest_cache_measure' and
'digest_cache_appraise' is not sufficient to use the Integrity Digest Cache
to measure and appraise files, since their effect is only to measure and
appraise digest lists.

Modify existing measurement rules if the 'digest_cache_measure' built-in
policy is specified by adding to them:

digest_cache=data pcr=12

Other than enabling the usage of the Integrity Digest Cache for
measurement, the additional keywords also store measurements in the PCR 12,
to not confuse new style measurements with the original ones still stored
on PCR 10.

Modify existing appraisal rules if the 'digest_cache_appraise' built-in
policy is specified by adding to them:

digest_cache=data

The additional keyword enables the usage of Integrity Digest Cache for
appraisal.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  9 +++++++--
 security/integrity/ima/ima_policy.c             | 14 ++++++++++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index afaaf125a237..1df8ce411760 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2101,11 +2101,16 @@
 			critical data.
 
 			The "digest_cache_measure" policy measures digest lists
-			into PCR 12 (can be changed with kernel config).
+			into PCR 12 (can be changed with kernel config), enables
+			the digest cache to be used for the other selected
+			measure rules (if compatible), and measures the files
+			with digest not found in the digest list into PCR 12
+			(changeable).
 
 			The "digest_cache_appraise" policy appraises digest
 			lists with IMA signatures or module-style appended
-			signatures.
+			signatures, and enables the digest cache to be used for
+			the other selected appraise rules (if compatible).
 
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c2bf58010b89..c74ff36fcc1f 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -930,6 +930,20 @@ static void add_rules(struct ima_rule_entry *entries, int count,
 	for (i = 0; i < count; i++) {
 		struct ima_rule_entry *entry;
 
+		if (IS_ENABLED(CONFIG_INTEGRITY_DIGEST_CACHE) &&
+		    entries[i].action == MEASURE && ima_digest_cache_measure &&
+		    ima_digest_cache_func_allowed(entries[i].func)) {
+			entries[i].digest_cache_usage |= IMA_DIGEST_CACHE_MEASURE_DATA;
+			entries[i].pcr = CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX;
+			entries[i].flags |= IMA_PCR;
+		}
+
+		if (IS_ENABLED(CONFIG_INTEGRITY_DIGEST_CACHE) &&
+		    entries[i].action == APPRAISE &&
+		    ima_digest_cache_appraise &&
+		    ima_digest_cache_func_allowed(entries[i].func))
+			entries[i].digest_cache_usage |= IMA_DIGEST_CACHE_APPRAISE_DATA;
+
 		if (policy_rule & IMA_DEFAULT_POLICY)
 			list_add_tail(&entries[i].list, &ima_default_rules);
 
-- 
2.34.1

[RFC][PATCH v3 06/10] ima: Retrieve digest cache and check if changed

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Add a digest cache pointer to the ima_iint_cache structure and introduce
ima_digest_cache_get_check() to retrieve a fresh digest cache and compare
with the pointer stored in the previous calls (if digest cache was enabled
in the IMA policy).

If the pointers don't match, reset the integrity status since the digest
cache used for the previous verification might have changed.

Also, initialize and put the digest cache respectively in
ima_iint_init_always() and ima_iint_free().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/Makefile           |  1 +
 security/integrity/ima/ima.h              |  1 +
 security/integrity/ima/ima_digest_cache.c | 49 +++++++++++++++++++++++
 security/integrity/ima/ima_digest_cache.h | 22 ++++++++++
 security/integrity/ima/ima_iint.c         |  4 ++
 security/integrity/ima/ima_main.c         | 10 ++++-
 6 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 security/integrity/ima/ima_digest_cache.c
 create mode 100644 security/integrity/ima/ima_digest_cache.h

diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index b376d38b4ee6..b4a284634a07 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -14,6 +14,7 @@ ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 ima-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
 ima-$(CONFIG_IMA_MEASURE_ASYMMETRIC_KEYS) += ima_asymmetric_keys.o
 ima-$(CONFIG_IMA_QUEUE_EARLY_BOOT_KEYS) += ima_queue_keys.o
+ima-$(CONFIG_INTEGRITY_DIGEST_CACHE) += ima_digest_cache.o
 
 ifeq ($(CONFIG_EFI),y)
 ima-$(CONFIG_IMA_SECURE_AND_OR_TRUSTED_BOOT) += ima_efi.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 736fe014afbc..b2ef37a11b65 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -198,6 +198,7 @@ struct ima_iint_cache {
 	enum integrity_status ima_read_status:4;
 	enum integrity_status ima_creds_status:4;
 	struct ima_digest_data *ima_hash;
+	struct digest_cache *digest_cache;
 };
 
 extern struct lsm_blob_sizes ima_blob_sizes;
diff --git a/security/integrity/ima/ima_digest_cache.c b/security/integrity/ima/ima_digest_cache.c
new file mode 100644
index 000000000000..bf720684b7b1
--- /dev/null
+++ b/security/integrity/ima/ima_digest_cache.c
@@ -0,0 +1,49 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Integrate with the Integrity Digest Cache.
+ */
+
+#include <linux/digest_cache.h>
+
+#include "ima_digest_cache.h"
+
+/**
+ * ima_digest_cache_get_check - Get digest cache and check if changed
+ * @dentry: Dentry of the inode for which the digest cache will be used
+ * @iint: Inode integrity metadata
+ *
+ * Get a digest cache for the dentry parameter and compare with the digest
+ * cache stored in the inode integrity metadata.
+ *
+ * It must be called with the iint->mutex held.
+ *
+ * Return: True if the digest cache pointer changed, false otherwise.
+ */
+bool ima_digest_cache_get_check(struct dentry *dentry,
+				struct ima_iint_cache *iint)
+{
+	struct digest_cache *digest_cache;
+
+	digest_cache = digest_cache_get(dentry);
+
+	/* There was no digest cache before, not changed. */
+	if (!iint->digest_cache) {
+		iint->digest_cache = digest_cache;
+		return false;
+	}
+
+	/* New digest cache not available, or digest cache changed. */
+	if (!digest_cache || iint->digest_cache != digest_cache) {
+		digest_cache_put(iint->digest_cache);
+		iint->digest_cache = digest_cache;
+		return true;
+	}
+
+	/* Digest cache not changed. */
+	digest_cache_put(digest_cache);
+	return false;
+}
diff --git a/security/integrity/ima/ima_digest_cache.h b/security/integrity/ima/ima_digest_cache.h
new file mode 100644
index 000000000000..75b6ac8732d3
--- /dev/null
+++ b/security/integrity/ima/ima_digest_cache.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2024 Huawei Technologies Duesseldorf GmbH
+ *
+ * Author: Roberto Sassu <roberto.sassu@huawei.com>
+ *
+ * Header file of ima_digest_cache.c.
+ */
+
+#include "ima.h"
+
+#ifdef CONFIG_INTEGRITY_DIGEST_CACHE
+bool ima_digest_cache_get_check(struct dentry *dentry,
+				struct ima_iint_cache *iint);
+#else
+static inline bool ima_digest_cache_get_check(struct dentry *dentry,
+					      struct ima_iint_cache *iint)
+{
+	return false;
+}
+
+#endif /* CONFIG_INTEGRITY_DIGEST_CACHE */
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index e23412a2c56b..d6946cd4fa91 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -68,12 +68,16 @@ static void ima_iint_init_always(struct ima_iint_cache *iint,
 	iint->ima_read_status = INTEGRITY_UNKNOWN;
 	iint->ima_creds_status = INTEGRITY_UNKNOWN;
 	iint->measured_pcrs = 0;
+	iint->digest_cache = NULL;
 	mutex_init(&iint->mutex);
 	ima_iint_lockdep_annotate(iint, inode);
 }
 
 static void ima_iint_free(struct ima_iint_cache *iint)
 {
+	if (iint->digest_cache)
+		digest_cache_put(iint->digest_cache);
+
 	kfree(iint->ima_hash);
 	mutex_destroy(&iint->mutex);
 	kmem_cache_free(ima_iint_cache, iint);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ebf233664194..ae106f1fe144 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -224,6 +224,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	bool violation_check;
 	enum hash_algo hash_algo;
 	unsigned int allowed_algos = 0;
+	u64 policy_usage = 0ULL;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -234,7 +235,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	 */
 	action = ima_get_action(file_mnt_idmap(file), inode, cred, secid,
 				mask, func, &pcr, &template_desc, NULL,
-				&allowed_algos, NULL);
+				&allowed_algos, &policy_usage);
 	violation_check = ((func == FILE_CHECK || func == MMAP_CHECK ||
 			    func == MMAP_CHECK_REQPROT) &&
 			   (ima_policy_flag & IMA_MEASURE));
@@ -287,6 +288,13 @@ static int process_measurement(struct file *file, const struct cred *cred,
 		iint->measured_pcrs = 0;
 	}
 
+	/* Digest cache changed, reset integrity status. */
+	if (policy_usage &&
+	    ima_digest_cache_get_check(file_dentry(file), iint)) {
+		iint->flags &= ~IMA_DONE_MASK;
+		iint->measured_pcrs = 0;
+	}
+
 	/*
 	 * On stacked filesystems, detect and re-evaluate file data and
 	 * metadata changes.
-- 
2.34.1

[RFC][PATCH v3 07/10] ima: Store verified usage in digest cache based on integrity metadata flags

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

The Integrity Digest Cache allows integrity providers to record how the
digest list being used to populate the digest cache was verified.

Integrity providers can register a kernel_post_read_file LSM hook
implementation, and call digest_cache_verif_set() providing the result of
the digest list verification, together with the digest list file
descriptor.

IMA implements ima_digest_cache_store_verified_usage(), storing verified
usage of the digest cache based on whether or not the digest list the
digest cache is being populated from was measured/appraised.

If the digest list was measured (IMA_MEASURED set in iint->flags),
ima_digest_cache_store_verified_usage() sets the
IMA_DIGEST_CACHE_MEASURE_DATA in the verified usage. If the digest list was
appraised (IMA_APPRAISED_SUBMASK), ima_digest_cache_store_verified_usage()
sets the IMA_DIGEST_CACHE_APPRAISE_DATA in the verified usage.

Verified usage based on integrity metadata will be ANDed with the policy
usage from the IMA policy. Then, the final decision will ultimately depend
on whether or not the calculated digest of the accessed file was found in
the digest cache.

ANDing the verified usage with the policy usage prevents remote verifiers
from receiving an incomplete IMA measurement list, where measurements are
skipped, but there isn't the digest list the calculated file digest was
searched into. It also prevents successful appraisal without appraising the
digest list itself.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_digest_cache.c | 35 +++++++++++++++++++++++
 security/integrity/ima/ima_digest_cache.h |  7 +++++
 security/integrity/ima/ima_main.c         |  3 ++
 3 files changed, 45 insertions(+)

diff --git a/security/integrity/ima/ima_digest_cache.c b/security/integrity/ima/ima_digest_cache.c
index bf720684b7b1..baa3faae77d2 100644
--- a/security/integrity/ima/ima_digest_cache.c
+++ b/security/integrity/ima/ima_digest_cache.c
@@ -47,3 +47,38 @@ bool ima_digest_cache_get_check(struct dentry *dentry,
 	digest_cache_put(digest_cache);
 	return false;
 }
+
+/**
+ * ima_digest_cache_store_verified_usage - Store verified usage in digest cache
+ * @file: Digest list file descriptor
+ * @iint: Inode integrity metadata
+ *
+ * Set digest cache verified usage in the digest cache associated to the
+ * digest list file descriptor. Verified usage is based on whether or not the
+ * digest list was measured/appraised, and is ANDed with the policy usage to
+ * make the final decision on whether a digest cache can be used for a specific
+ * IMA action.
+ */
+void ima_digest_cache_store_verified_usage(struct file *file,
+					   struct ima_iint_cache *iint)
+{
+	u64 verified_usage = 0;
+	int rc;
+
+	if (iint->flags & IMA_MEASURED)
+		verified_usage |= IMA_DIGEST_CACHE_MEASURE_DATA;
+	if (iint->flags & IMA_APPRAISED_SUBMASK)
+		verified_usage |= IMA_DIGEST_CACHE_APPRAISE_DATA;
+
+	/*
+	 * Set digest cache verified usage from integrity metadata flags for
+	 * later use.
+	 */
+	rc = digest_cache_verif_set(file, "ima", &verified_usage,
+				    sizeof(verified_usage));
+
+	/* Ignore if fd doesn't have digest cache set (prefetching). */
+	if (rc && rc != -ENOENT)
+		pr_debug("Cannot set verified usage for %s, ret: %d, ignoring\n",
+			 file_dentry(file)->d_name.name, rc);
+}
diff --git a/security/integrity/ima/ima_digest_cache.h b/security/integrity/ima/ima_digest_cache.h
index 75b6ac8732d3..1544dc798631 100644
--- a/security/integrity/ima/ima_digest_cache.h
+++ b/security/integrity/ima/ima_digest_cache.h
@@ -12,6 +12,8 @@
 #ifdef CONFIG_INTEGRITY_DIGEST_CACHE
 bool ima_digest_cache_get_check(struct dentry *dentry,
 				struct ima_iint_cache *iint);
+void ima_digest_cache_store_verified_usage(struct file *file,
+					   struct ima_iint_cache *iint);
 #else
 static inline bool ima_digest_cache_get_check(struct dentry *dentry,
 					      struct ima_iint_cache *iint)
@@ -19,4 +21,9 @@ static inline bool ima_digest_cache_get_check(struct dentry *dentry,
 	return false;
 }
 
+static inline void
+ima_digest_cache_store_verified_usage(struct file *file,
+				      struct ima_iint_cache *iint)
+{ }
+
 #endif /* CONFIG_INTEGRITY_DIGEST_CACHE */
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index ae106f1fe144..97ece2abb0b9 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -29,6 +29,7 @@
 #include <linux/evm.h>
 
 #include "ima.h"
+#include "ima_digest_cache.h"
 
 #ifdef CONFIG_IMA_APPRAISE
 int ima_appraise = IMA_APPRAISE_ENFORCE;
@@ -420,6 +421,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) &&
 	     !(iint->flags & IMA_NEW_FILE))
 		rc = -EACCES;
+	if (!rc && func == DIGEST_LIST_CHECK)
+		ima_digest_cache_store_verified_usage(file, iint);
 	mutex_unlock(&iint->mutex);
 	kfree(xattr_value);
 	ima_free_modsig(modsig);
-- 
2.34.1

[RFC][PATCH v3 08/10] ima: Load verified usage from digest cache found from query

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce ima_digest_cache_load_verified_usage() to retrieve the verified
usage from the digest cache returned from the lookup of the digest
calculated by IMA on a file being accessed.

Verified usage cannot be loaded from the digest cache returned by
digest_cache_get() since the latter might return a directory digest cache,
which does not contain any verification data (only set to digest cache
populated from a file).

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima_digest_cache.c | 37 +++++++++++++++++++++++
 security/integrity/ima/ima_digest_cache.h |  9 ++++++
 2 files changed, 46 insertions(+)

diff --git a/security/integrity/ima/ima_digest_cache.c b/security/integrity/ima/ima_digest_cache.c
index baa3faae77d2..bb8a9a9aa933 100644
--- a/security/integrity/ima/ima_digest_cache.c
+++ b/security/integrity/ima/ima_digest_cache.c
@@ -82,3 +82,40 @@ void ima_digest_cache_store_verified_usage(struct file *file,
 		pr_debug("Cannot set verified usage for %s, ret: %d, ignoring\n",
 			 file_dentry(file)->d_name.name, rc);
 }
+
+/**
+ * ima_digest_cache_load_verified_usage - Load verified usage from digest cache
+ * @dentry: Dentry of the inode for which the digest cache will be used
+ * @iint: Inode integrity metadata
+ *
+ * Load digest cache verified usage from the digest cache returned by
+ * digest_cache_lookup(), containing the file digest calculated by IMA (if the
+ * digest is found).
+ *
+ * Return: Verified usage if digest is found in digest cache, zero otherwise.
+ */
+u64 ima_digest_cache_load_verified_usage(struct dentry *dentry,
+					 struct ima_iint_cache *iint)
+{
+	void *verified_usage_ptr;
+	uintptr_t found_cache_ptr;
+	struct digest_cache *found_cache;
+
+	if (!iint->digest_cache)
+		return 0ULL;
+
+	/* Do lookup to identify digest cache verification data is read from. */
+	found_cache_ptr = digest_cache_lookup(dentry, iint->digest_cache,
+					      iint->ima_hash->digest,
+					      iint->ima_hash->algo);
+	if (!found_cache_ptr)
+		return 0ULL;
+
+	found_cache = (struct digest_cache *)found_cache_ptr;
+
+	verified_usage_ptr = digest_cache_verif_get(found_cache, "ima");
+	if (!verified_usage_ptr)
+		return 0ULL;
+
+	return *(u64 *)verified_usage_ptr;
+}
diff --git a/security/integrity/ima/ima_digest_cache.h b/security/integrity/ima/ima_digest_cache.h
index 1544dc798631..41a37630b1f7 100644
--- a/security/integrity/ima/ima_digest_cache.h
+++ b/security/integrity/ima/ima_digest_cache.h
@@ -14,6 +14,8 @@ bool ima_digest_cache_get_check(struct dentry *dentry,
 				struct ima_iint_cache *iint);
 void ima_digest_cache_store_verified_usage(struct file *file,
 					   struct ima_iint_cache *iint);
+u64 ima_digest_cache_load_verified_usage(struct dentry *dentry,
+					 struct ima_iint_cache *iint);
 #else
 static inline bool ima_digest_cache_get_check(struct dentry *dentry,
 					      struct ima_iint_cache *iint)
@@ -26,4 +28,11 @@ ima_digest_cache_store_verified_usage(struct file *file,
 				      struct ima_iint_cache *iint)
 { }
 
+static inline u64
+ima_digest_cache_load_verified_usage(struct dentry *dentry,
+				     struct ima_iint_cache *iint)
+{
+	return 0ULL;
+}
+
 #endif /* CONFIG_INTEGRITY_DIGEST_CACHE */
-- 
2.34.1

[RFC][PATCH v3 09/10] ima: Use digest caches for measurement

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Introduce a new measurement style using digest caches, which can be
performed exclusively on non-standard PCRs, to avoid ambiguity.

While a measurement on the standard PCR means that a file was accessed and
had the measured data, a measurement with the digest cache means only that
the calculated file digest was not found in any of the measured digest
lists (any digest list used for the search must be measured, otherwise IMA
wouldn't use it).

The new measurement style does not tell: whether or not the file was
actually accessed (since its measurement is skipped even if it was); in
which sequence files were accessed. So, one has to guess that the system
might have possibly accessed any of the files whose digest is in the
measured digest lists, in any order.

However, it has the following benefits: the IMA measurement list can be
much shorter, system performance can be much better due to less PCR extend
operations (see the performance evaluation in the Integrity Digest Cache
documentation); the PCR can be predictable as long as the set of measured
digest lists does not change (which obviously happens during software
updates).

The PCR can be predictable because the Integrity Digest Cache has a
prefetching mechanism that reads digest lists in a deterministic order,
until it finds the digest list containing the digest calculated by IMA from
an accessed file. If IMA measures digest lists, the PCR is extended in a
deterministic order too.

Predictable PCR means that a TPM key can be made dependent on specific PCR
values (or a OR of them, depending on the key policy). Accessing a file
with an unknown digest immediately makes that TPM key unusable, requiring a
reboot to use it again.

This mechanism can be used for the so called implicit remote attestation,
where the ability of a system to respond to challenges based on the private
part of the TPM key means that the system has the expected PCR values
(which would mean that the integrity of the system is ok). This is opposed
to the explicit remote attestation, where a system has to send all its
measurements, to prove to a remote party about its integrity.

Call the newly introduced function ima_digest_cache_load_verified_usage()
to retrieve the verified usage from the digest cache containing the
calculated digest of the file being accessed (if it is found), and AND it
with the policy usage.

If the AND result has the IMA_DIGEST_CACHE_MEASURE_DATA flag set, behave as
if the file was successfully added to the IMA measurement list (i.e. set
the IMA_MEASURED flag and the PCR flag from the value specified in the
matching policy rule), but actually don't do it.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h      |  3 ++-
 security/integrity/ima/ima_api.c  | 15 ++++++++++++++-
 security/integrity/ima/ima_main.c |  8 ++++++--
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index b2ef37a11b65..c915339dd0d4 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -389,7 +389,8 @@ void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
-			   struct ima_template_desc *template_desc);
+			   struct ima_template_desc *template_desc,
+			   u64 allowed_usage);
 int process_buffer_measurement(struct mnt_idmap *idmap,
 			       struct inode *inode, const void *buf, int size,
 			       const char *eventname, enum ima_hooks func,
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index b44cf7d9fbcb..530c5bcc115e 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -351,7 +351,8 @@ void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
 			   const unsigned char *filename,
 			   struct evm_ima_xattr_data *xattr_value,
 			   int xattr_len, const struct modsig *modsig, int pcr,
-			   struct ima_template_desc *template_desc)
+			   struct ima_template_desc *template_desc,
+			   u64 allowed_usage)
 {
 	static const char op[] = "add_template_measure";
 	static const char audit_cause[] = "ENOMEM";
@@ -375,6 +376,18 @@ void ima_store_measurement(struct ima_iint_cache *iint, struct file *file,
 	if (iint->measured_pcrs & (0x1 << pcr) && !modsig)
 		return;
 
+	/*
+	 * If digest cache usage was authorized with the IMA policy, the digest
+	 * list the digest cache was populated from was measured, and the file
+	 * digest was found in the digest cache, mark the file as successfully
+	 * measured.
+	 */
+	if (allowed_usage & IMA_DIGEST_CACHE_MEASURE_DATA) {
+		iint->flags |= IMA_MEASURED;
+		iint->measured_pcrs |= (0x1 << pcr);
+		return;
+	}
+
 	result = ima_alloc_init_template(&event_data, &entry, template_desc);
 	if (result < 0) {
 		integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 97ece2abb0b9..3b34bfa5c891 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -225,7 +225,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	bool violation_check;
 	enum hash_algo hash_algo;
 	unsigned int allowed_algos = 0;
-	u64 policy_usage = 0ULL;
+	u64 policy_usage = 0ULL, verified_usage = 0ULL;
 
 	if (!ima_policy_flag || !S_ISREG(inode->i_mode))
 		return 0;
@@ -385,10 +385,14 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
 
+	verified_usage = ima_digest_cache_load_verified_usage(file_dentry(file),
+							      iint);
+
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, modsig, pcr,
-				      template_desc);
+				      template_desc,
+				      (policy_usage & verified_usage));
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		rc = ima_check_blacklist(iint, modsig, pcr);
 		if (rc != -EPERM) {
-- 
2.34.1

[RFC][PATCH v3 10/10] ima: Use digest caches for appraisal

[Roberto Sassu]

From: Roberto Sassu <roberto.sassu@huawei.com>

Similarly to measurement, enable the new appraisal style too using digest
caches.

It allows files to be verified by only checking the signature of the
package they belong to (as opposed to checking individual file signatures),
and by doing a digest lookup in the digest cache.

The first benefit is that packages are already signed (e.g. RPM package
headers) and also contain file digests, so it is not necessary to further
add signatures for each file.

The second benefit is performance, which improves due to verifying less
signatures (computationally expensive).

Appraisal with a digest cache is successful only if the allowed usage
(AND of policy usage and verified usage) has the
IMA_DIGEST_CACHE_APPRAISE_DATA bit set, and if EVM is disabled or the file
does not have any protected xattr

The digest cache appraisal method is tried first, for performance reasons.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h          |  6 ++++--
 security/integrity/ima/ima_appraise.c | 31 +++++++++++++++++++++------
 security/integrity/ima/ima_main.c     |  4 +++-
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index c915339dd0d4..8f74783e6174 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -440,7 +440,8 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
 int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, const struct modsig *modsig);
+			     int xattr_len, const struct modsig *modsig,
+			     u64 allowed_usage);
 int ima_must_appraise(struct mnt_idmap *idmap, struct inode *inode,
 		      int mask, enum ima_hooks func);
 void ima_update_xattr(struct ima_iint_cache *iint, struct file *file);
@@ -465,7 +466,8 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
 					   int xattr_len,
-					   const struct modsig *modsig)
+					   const struct modsig *modsig,
+					   u64 allowed_usage)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index d479cf58d859..d4745d90ae23 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -480,7 +480,8 @@ int ima_check_blacklist(struct ima_iint_cache *iint,
 int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, const struct modsig *modsig)
+			     int xattr_len, const struct modsig *modsig,
+			     u64 allowed_usage)
 {
 	static const char op[] = "appraise_data";
 	const char *cause = "unknown";
@@ -489,13 +490,20 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len;
 	bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
+	bool cache_pass = (allowed_usage & IMA_DIGEST_CACHE_APPRAISE_DATA);
 
-	/* If not appraising a modsig, we need an xattr. */
-	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig)
+	/*
+	 * If not appraising a modsig/there is no digest cache match, we need
+	 * an xattr.
+	 */
+	if (!(inode->i_opflags & IOP_XATTR) && !try_modsig && !cache_pass)
 		return INTEGRITY_UNKNOWN;
 
-	/* If reading the xattr failed and there's no modsig, error out. */
-	if (rc <= 0 && !try_modsig) {
+	/*
+	 * If reading the xattr failed and there's no modsig/digest cache match,
+	 * error out.
+	 */
+	if (rc <= 0 && !try_modsig && !cache_pass) {
 		if (rc && rc != -ENODATA)
 			goto out;
 
@@ -526,8 +534,11 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 	case INTEGRITY_UNKNOWN:
 		break;
 	case INTEGRITY_NOXATTRS:	/* No EVM protected xattrs. */
-		/* It's fine not to have xattrs when using a modsig. */
-		if (try_modsig)
+		/*
+		 * It's fine not to have xattrs when using a modsig or there
+		 * is a digest cache match.
+		 */
+		if (try_modsig || cache_pass)
 			break;
 		fallthrough;
 	case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
@@ -544,6 +555,12 @@ int ima_appraise_measurement(enum ima_hooks func, struct ima_iint_cache *iint,
 		WARN_ONCE(true, "Unexpected integrity status %d\n", status);
 	}
 
+	if (cache_pass) {
+		status = INTEGRITY_PASS;
+		rc = 0;
+		goto out;
+	}
+
 	if (xattr_value)
 		rc = xattr_verify(func, iint, xattr_value, xattr_len, &status,
 				  &cause);
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 3b34bfa5c891..aa2f117ae0ba 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -399,7 +399,9 @@ static int process_measurement(struct file *file, const struct cred *cred,
 			inode_lock(inode);
 			rc = ima_appraise_measurement(func, iint, file,
 						      pathname, xattr_value,
-						      xattr_len, modsig);
+						      xattr_len, modsig,
+						      (policy_usage &
+						       verified_usage));
 			inode_unlock(inode);
 		}
 		if (!rc)
-- 
2.34.1

Re: [RFC][PATCH v3 01/10] ima: Introduce hook DIGEST_LIST_CHECK

[Jarkko Sakkinen]

On Thu Sep 5, 2024 at 6:25 PM EEST, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Introduce a new hook to check the integrity of digest lists.

"Introduce DIGEST_LIST_CHECK, a new hook..."

>
> The new hook is invoked during a kernel read with file type

"with the file type"


> READING_DIGEST LIST, which is done by the Integrity Digest Cache when it is
> populating a digest cache with a digest list.

The patch creates a new struct imap_rule_entry instance when it parses
the corresponding rule, which means that there are couple slices of
information missing here:

1. The commit message does not tell what the code change effectively
   is. I scavenged this information from [1].
2. The commit message does no effort to connect the dots between the
   effective change and the expected goal.

I'd put a lot of effort to this commit message assuming that the new
hook is at the center of the goals of this patch set.

[1] https://elixir.bootlin.com/linux/v6.10-rc4/source/security/integrity/ima/ima_policy.c#L1404

BR, Jarkko

Re: [RFC][PATCH v3 04/10] ima: Add digest_cache_measure/appraise boot-time built-in policies

[Jarkko Sakkinen]

On Thu Sep 5, 2024 at 6:25 PM EEST, Roberto Sassu wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
>
> Specify the 'digest_cache_measure' boot-time policy with 'ima_policy=' in
> the kernel command line to add the following rule at the beginning of the
> IMA policy, before other rules:
>
> measure func=DIGEST_LIST_CHECK pcr=12
>
> which will measure digest lists into PCR 12 (or the value in
> CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX).
>
> Specify 'digest_cache_appraise' to add the following rule at the beginning,
> before other rules:
>
> appraise func=DIGEST_LIST_CHECK appraise_type=imasig|modsig
>
> which will appraise digest lists with IMA signatures or module-style
> appended signatures.
>
> Adding those rule at the beginning rather than at the end is necessary to
> ensure that digest lists are measured and appraised in the initial ram
> disk, which would be otherwise prevented by the dont_ rules.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
>  .../admin-guide/kernel-parameters.txt         | 10 +++++-
>  security/integrity/ima/Kconfig                | 10 ++++++
>  security/integrity/ima/ima_policy.c           | 35 +++++++++++++++++++
>  3 files changed, 54 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 09126bb8cc9f..afaaf125a237 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2077,7 +2077,8 @@
>  	ima_policy=	[IMA]
>  			The builtin policies to load during IMA setup.
>  			Format: "tcb | appraise_tcb | secure_boot |
> -				 fail_securely | critical_data"
> +				 fail_securely | critical_data |
> +				 digest_cache_measure | digest_cache_appraise"
>  
>  			The "tcb" policy measures all programs exec'd, files
>  			mmap'd for exec, and all files opened with the read
> @@ -2099,6 +2100,13 @@
>  			The "critical_data" policy measures kernel integrity
>  			critical data.
>  
> +			The "digest_cache_measure" policy measures digest lists
> +			into PCR 12 (can be changed with kernel config).
> +
> +			The "digest_cache_appraise" policy appraises digest
> +			lists with IMA signatures or module-style appended
> +			signatures.
> +
>  	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
>  			Load a policy which meets the needs of the Trusted
>  			Computing Base.  This means IMA will measure all

Must be updated as a separate commit as kernel-parameters.txt not
part of IMA. Consider it as a different subsystem in this context.

BR, Jarkko

Re: [RFC][PATCH v3 01/10] ima: Introduce hook DIGEST_LIST_CHECK

[Roberto Sassu]

On Fri, 2024-09-06 at 12:41 +0300, Jarkko Sakkinen wrote:
> On Thu Sep 5, 2024 at 6:25 PM EEST, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Introduce a new hook to check the integrity of digest lists.
> 
> "Introduce DIGEST_LIST_CHECK, a new hook..."
> 
> > 
> > The new hook is invoked during a kernel read with file type
> 
> "with the file type"
> 
> 
> > READING_DIGEST LIST, which is done by the Integrity Digest Cache when it is
> > populating a digest cache with a digest list.
> 
> The patch creates a new struct imap_rule_entry instance when it parses
> the corresponding rule, which means that there are couple slices of
> information missing here:
> 
> 1. The commit message does not tell what the code change effectively
>    is. I scavenged this information from [1].

Sorry, to me it seems a bit redundant to state what a IMA hook is. The
new hook will be handled by IMA like the other existing hooks.

> 2. The commit message does no effort to connect the dots between the
>    effective change and the expected goal.

Sure, will mention the goal better.

Thanks

Roberto

> I'd put a lot of effort to this commit message assuming that the new
> hook is at the center of the goals of this patch set.
> 
> [1] https://elixir.bootlin.com/linux/v6.10-rc4/source/security/integrity/ima/ima_policy.c#L1404
> 
> BR, Jarkko

Re: [RFC][PATCH v3 04/10] ima: Add digest_cache_measure/appraise boot-time built-in policies

[Roberto Sassu]

On Fri, 2024-09-06 at 12:45 +0300, Jarkko Sakkinen wrote:
> On Thu Sep 5, 2024 at 6:25 PM EEST, Roberto Sassu wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> > 
> > Specify the 'digest_cache_measure' boot-time policy with 'ima_policy=' in
> > the kernel command line to add the following rule at the beginning of the
> > IMA policy, before other rules:
> > 
> > measure func=DIGEST_LIST_CHECK pcr=12
> > 
> > which will measure digest lists into PCR 12 (or the value in
> > CONFIG_IMA_DIGEST_CACHE_MEASURE_PCR_IDX).
> > 
> > Specify 'digest_cache_appraise' to add the following rule at the beginning,
> > before other rules:
> > 
> > appraise func=DIGEST_LIST_CHECK appraise_type=imasig|modsig
> > 
> > which will appraise digest lists with IMA signatures or module-style
> > appended signatures.
> > 
> > Adding those rule at the beginning rather than at the end is necessary to
> > ensure that digest lists are measured and appraised in the initial ram
> > disk, which would be otherwise prevented by the dont_ rules.
> > 
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > ---
> >  .../admin-guide/kernel-parameters.txt         | 10 +++++-
> >  security/integrity/ima/Kconfig                | 10 ++++++
> >  security/integrity/ima/ima_policy.c           | 35 +++++++++++++++++++
> >  3 files changed, 54 insertions(+), 1 deletion(-)
> > 
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> > index 09126bb8cc9f..afaaf125a237 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2077,7 +2077,8 @@
> >  	ima_policy=	[IMA]
> >  			The builtin policies to load during IMA setup.
> >  			Format: "tcb | appraise_tcb | secure_boot |
> > -				 fail_securely | critical_data"
> > +				 fail_securely | critical_data |
> > +				 digest_cache_measure | digest_cache_appraise"
> >  
> >  			The "tcb" policy measures all programs exec'd, files
> >  			mmap'd for exec, and all files opened with the read
> > @@ -2099,6 +2100,13 @@
> >  			The "critical_data" policy measures kernel integrity
> >  			critical data.
> >  
> > +			The "digest_cache_measure" policy measures digest lists
> > +			into PCR 12 (can be changed with kernel config).
> > +
> > +			The "digest_cache_appraise" policy appraises digest
> > +			lists with IMA signatures or module-style appended
> > +			signatures.
> > +
> >  	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
> >  			Load a policy which meets the needs of the Trusted
> >  			Computing Base.  This means IMA will measure all
> 
> Must be updated as a separate commit as kernel-parameters.txt not
> part of IMA. Consider it as a different subsystem in this context.

Can be done, but this would be more like an atomic change in case the
patch is reverted.

Thanks

Roberto

Re: [RFC][PATCH v3 01/10] ima: Introduce hook DIGEST_LIST_CHECK

[Jarkko Sakkinen]

On Fri Sep 6, 2024 at 2:22 PM EEST, Roberto Sassu wrote:
> On Fri, 2024-09-06 at 12:41 +0300, Jarkko Sakkinen wrote:
> > On Thu Sep 5, 2024 at 6:25 PM EEST, Roberto Sassu wrote:
> > > From: Roberto Sassu <roberto.sassu@huawei.com>
> > > 
> > > Introduce a new hook to check the integrity of digest lists.
> > 
> > "Introduce DIGEST_LIST_CHECK, a new hook..."
> > 
> > > 
> > > The new hook is invoked during a kernel read with file type
> > 
> > "with the file type"
> > 
> > 
> > > READING_DIGEST LIST, which is done by the Integrity Digest Cache when it is
> > > populating a digest cache with a digest list.
> > 
> > The patch creates a new struct imap_rule_entry instance when it parses
> > the corresponding rule, which means that there are couple slices of
> > information missing here:
> > 
> > 1. The commit message does not tell what the code change effectively
> >    is. I scavenged this information from [1].
>
> Sorry, to me it seems a bit redundant to state what a IMA hook is. The
> new hook will be handled by IMA like the other existing hooks.

I think with documentation (scoping also to commit messages) it is in
general a good strategy to put it less rather than more. No
documentation is better than polluted documentation ;-)

Just remarking what might not be obvious with someone who might not
be obvious, unless being a pro-active contributor.

BR, Jarkko