Right now, PCR banks with unsupported hash algorithms are getting
invalidated over and over again for each new measurement list entry
recorded.
A subsequent patch will make IMA to invalidate PCR banks associated with
unsupported hash algorithms only once at a PCR's first use. To prepare for
that, make it track the set of PCRs ever extended.
Maintain the set of touched PCRs in an unsigned long bitmask,
'ima_extended_pcrs_mask'.
Amend the IMA_INVALID_PCR() #define to check that a given PCR can get
represented in that bitmask. Note that this is only for improving code
maintainablity, it does not actually constain the set of allowed PCR
indices any further.
Make ima_pcr_extend() to maintain the ima_extended_pcrs_mask, i.e. to set
the currently extented PCR's corresponding bit.
Note that at this point there's no provision to restore the
ima_extended_pcrs_mask value after kexecs yet, that will be the subject of
later patches.
Signed-off-by: Nicolai Stange <nstange@suse.de>
---
security/integrity/ima/ima.h | 8 ++++++--
security/integrity/ima/ima_queue.c | 17 +++++++++++++----
2 files changed, 19 insertions(+), 6 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 1158a7b8bf6b..f99b1f81b35c 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -20,6 +20,7 @@
#include <linux/hash.h>
#include <linux/tpm.h>
#include <linux/audit.h>
+#include <linux/minmax.h>
#include <crypto/hash_info.h>
#include "../integrity.h"
@@ -62,6 +63,8 @@ extern int ima_hash_algo_idx __ro_after_init;
extern int ima_extra_slots __ro_after_init;
extern struct ima_algo_desc *ima_algo_array __ro_after_init;
+extern unsigned long ima_extended_pcrs_mask;
+
extern int ima_appraise;
extern struct tpm_chip *ima_tpm_chip;
extern const char boot_aggregate_name[];
@@ -198,8 +201,9 @@ struct ima_iint_cache {
struct ima_digest_data *ima_hash;
};
-#define IMA_INVALID_PCR(a) (((a) < 0) || \
- (a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
+#define IMA_INVALID_PCR(a) (((a) < 0) || \
+ (a) >= (8 * min(sizeof_field(struct ima_iint_cache, measured_pcrs), \
+ sizeof(ima_extended_pcrs_mask))))
extern struct lsm_blob_sizes ima_blob_sizes;
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 0cc1189446a8..6e8a7514d9f6 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -51,6 +51,11 @@ static DEFINE_MUTEX(ima_extend_list_mutex);
*/
static bool ima_measurements_suspended;
+/*
+ * Set of PCRs ever extended by IMA.
+ */
+unsigned long ima_extended_pcrs_mask;
+
/* lookup up the digest value in the hash table, and return the entry */
static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
int pcr)
@@ -144,15 +149,19 @@ unsigned long ima_get_binary_runtime_size(void)
static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
{
- int result = 0;
+ int result;
if (!ima_tpm_chip)
- return result;
+ return 0;
result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
- if (result != 0)
+ if (result != 0) {
pr_err("Error Communicating to TPM chip, result: %d\n", result);
- return result;
+ return result;
+ }
+
+ ima_extended_pcrs_mask |= BIT(pcr);
+ return 0;
}
/*
--
2.49.0On Sun, 2025-03-23 at 15:09 +0100, Nicolai Stange wrote:
> Right now, PCR banks with unsupported hash algorithms are getting
> invalidated over and over again for each new measurement list entry
> recorded.
>
> A subsequent patch will make IMA to invalidate PCR banks associated with
> unsupported hash algorithms only once at a PCR's first use. To prepare for
> that, make it track the set of PCRs ever extended.
>
> Maintain the set of touched PCRs in an unsigned long bitmask,
> 'ima_extended_pcrs_mask'.
>
> Amend the IMA_INVALID_PCR() #define to check that a given PCR can get
> represented in that bitmask. Note that this is only for improving code
> maintainablity, it does not actually constain the set of allowed PCR
> indices any further.
>
> Make ima_pcr_extend() to maintain the ima_extended_pcrs_mask, i.e. to set
> the currently extented PCR's corresponding bit.
>
> Note that at this point there's no provision to restore the
> ima_extended_pcrs_mask value after kexecs yet, that will be the subject of
> later patches.
>
> Signed-off-by: Nicolai Stange <nstange@suse.de>
Hi Nicolai,
IMA extends measurements in the default TPM PCR based on the Kconfig
CONFIG_IMA_MEASURE_PCR_IDX option. Normally that is set to PCR 10. The IMA
policy rules may override the default PCR with a per policy rule specific PCR.
INVALID_PCR() checks the IMA policy rule specified is a valid PCR register.
Is the purpose of this patch to have a single per TPM bank violation or multiple
violations, one for each PCR used within the TPM bank?
thanks,
Mimi
> ---
> security/integrity/ima/ima.h | 8 ++++++--
> security/integrity/ima/ima_queue.c | 17 +++++++++++++----
> 2 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 1158a7b8bf6b..f99b1f81b35c 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -20,6 +20,7 @@
> #include <linux/hash.h>
> #include <linux/tpm.h>
> #include <linux/audit.h>
> +#include <linux/minmax.h>
> #include <crypto/hash_info.h>
>
> #include "../integrity.h"
> @@ -62,6 +63,8 @@ extern int ima_hash_algo_idx __ro_after_init;
> extern int ima_extra_slots __ro_after_init;
> extern struct ima_algo_desc *ima_algo_array __ro_after_init;
>
> +extern unsigned long ima_extended_pcrs_mask;
> +
> extern int ima_appraise;
> extern struct tpm_chip *ima_tpm_chip;
> extern const char boot_aggregate_name[];
> @@ -198,8 +201,9 @@ struct ima_iint_cache {
> struct ima_digest_data *ima_hash;
> };
>
> -#define IMA_INVALID_PCR(a) (((a) < 0) || \
> - (a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
> +#define IMA_INVALID_PCR(a) (((a) < 0) || \
> + (a) >= (8 * min(sizeof_field(struct ima_iint_cache, measured_pcrs), \
> + sizeof(ima_extended_pcrs_mask))))
>
>
> extern struct lsm_blob_sizes ima_blob_sizes;
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 0cc1189446a8..6e8a7514d9f6 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -51,6 +51,11 @@ static DEFINE_MUTEX(ima_extend_list_mutex);
> */
> static bool ima_measurements_suspended;
>
> +/*
> + * Set of PCRs ever extended by IMA.
> + */
> +unsigned long ima_extended_pcrs_mask;
> +
> /* lookup up the digest value in the hash table, and return the entry */
> static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
> int pcr)
> @@ -144,15 +149,19 @@ unsigned long ima_get_binary_runtime_size(void)
>
> static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
> {
> - int result = 0;
> + int result;
>
> if (!ima_tpm_chip)
> - return result;
> + return 0;
>
> result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
> - if (result != 0)
> + if (result != 0) {
> pr_err("Error Communicating to TPM chip, result: %d\n", result);
> - return result;
> + return result;
> + }
> +
> + ima_extended_pcrs_mask |= BIT(pcr);
> + return 0;
> }
>
> /*
Mimi Zohar <zohar@linux.ibm.com> writes: > On Sun, 2025-03-23 at 15:09 +0100, Nicolai Stange wrote: >> Right now, PCR banks with unsupported hash algorithms are getting >> invalidated over and over again for each new measurement list entry >> recorded. >> >> A subsequent patch will make IMA to invalidate PCR banks associated with >> unsupported hash algorithms only once at a PCR's first use. To prepare for >> that, make it track the set of PCRs ever extended. >> >> Maintain the set of touched PCRs in an unsigned long bitmask, >> 'ima_extended_pcrs_mask'. >> >> Amend the IMA_INVALID_PCR() #define to check that a given PCR can get >> represented in that bitmask. Note that this is only for improving code >> maintainablity, it does not actually constain the set of allowed PCR >> indices any further. >> >> Make ima_pcr_extend() to maintain the ima_extended_pcrs_mask, i.e. to set >> the currently extented PCR's corresponding bit. >> >> Note that at this point there's no provision to restore the >> ima_extended_pcrs_mask value after kexecs yet, that will be the subject of >> later patches. >> >> Signed-off-by: Nicolai Stange <nstange@suse.de> > > Hi Nicolai, > > IMA extends measurements in the default TPM PCR based on the Kconfig > CONFIG_IMA_MEASURE_PCR_IDX option. Normally that is set to PCR 10. The IMA > policy rules may override the default PCR with a per policy rule > specific PCR. Yes, that matches my understanding. > INVALID_PCR() checks the IMA policy rule specified is a valid PCR register. > > Is the purpose of this patch to have a single per TPM bank violation or multiple > violations, one for each PCR used within the TPM bank? One for each PCR individually, issued when a given PCR is being referenced for the first time from some IMA event. Thanks! Nicolai -- SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany GF: Ivo Totev, Andrew McDonald, Werner Knoblich (HRB 36809, AG Nürnberg)
© 2016 - 2025 Red Hat, Inc.