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 <nsta...@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;
> }
>
> /*
