[RFC PATCH v1 0/7] ima: get rid of hard dependency on SHA-1

[RFC PATCH v1 0/7] ima: get rid of hard dependency on SHA-1

[Nicolai Stange]

Hi all,

if no SHA-1 implementation was available to the kernel, IMA init would
currently fail, rendering the whole subsystem unusable.

This patch series is an attempt to make SHA-1 availability non-mandatory
for IMA. The main motivation is that NIST announced to sunset SHA-1 by
2030 ([1]), whereby any attempt to instantiate it when booted in FIPS mode
would have to be made to fail with -ENOENT. As this does potentially have
an impact on lifetimes for FIPS certifications issued today, distros might
be interested in disabling SHA-1 downstream soon already.

Anyway, making IMA to work without a SHA-1 implementation available is not
so straightforward, mainly due to that established scheme to substitute
padded SHA-1 template hashes for PCR banks with unmapped/unavailable algos.
There is some userspace around expecting that existing behavior, e.g. the
ima_measurement command from ([2]), and breaking that in certain scenarios
is inevitable.

I tried to make it the least painful possible, and I think I arrived at
a not completely unreasonable solution in the end, but wouldn't be too
surprised if you had a different stance on that. So I would be curious
about your feedback on whether this is a route worth pursuing any further.
FWIW, the most controversial parts are probably
 - [1/7] ima: don't expose runtime_measurements for unsupported hashes
 - [6/7] ima: invalidate unsupported PCR banks once at first use

Note that I haven't tested this series thoroughly yet -- for the time being
I only ran a couple of brief smoke tests in a VM w/o a TPM  (w/ and w/o
SHA-1 disabled of course).

Many thanks!

Nicolai

[1] https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm
[2] https://github.com/linux-integrity/ima-evm-utils.git

Nicolai Stange (7):
  ima: don't expose runtime_measurements for unsupported hashes
  ima: always create runtime_measurements sysfs file for ima_hash
  ima: move INVALID_PCR() to ima.h
  ima: track the set of PCRs ever extended
  tpm: enable bank selection for PCR extend
  ima: invalidate unsupported PCR banks once at first use
  ima: make SHA1 non-mandatory

 drivers/char/tpm/tpm-interface.c      | 29 +++++++++-
 drivers/char/tpm/tpm.h                |  3 +-
 drivers/char/tpm/tpm2-cmd.c           | 29 +++++++++-
 include/linux/tpm.h                   |  3 +
 security/integrity/ima/Kconfig        | 14 +++++
 security/integrity/ima/ima.h          |  9 +++
 security/integrity/ima/ima_crypto.c   | 83 ++++++++++++++++-----------
 security/integrity/ima/ima_fs.c       | 41 +++++++------
 security/integrity/ima/ima_policy.c   |  5 +-
 security/integrity/ima/ima_queue.c    | 26 ++++++++-
 security/integrity/ima/ima_template.c |  7 +++
 11 files changed, 190 insertions(+), 59 deletions(-)

-- 
2.47.1

[RFC PATCH v1 1/7] ima: don't expose runtime_measurements for unsupported hashes

[Nicolai Stange]

IMA creates one runtime_measurements_<hash-algo> sysfs file for every TPM
bank + for SHA1 if not covered by any such. These differ only in that the
template hash value for each record is of the file's associated algorithm
each.

The kernel does not necessarily support each hash algorithm associated
with some TPM bank though -- the most common case probably being that the
algorithm is not built-in, but provided as a module, if at all, and thus
not available at IMA init time yet.

If that happens to be the case, the behavior is a bit counter-intuitive:
probably for historic reasons and to still extend the TPM bank with
something, a record's template hash is filled with the padded SHA1 value.
That is, it is perfectly possible that runtime_measurements_sha256 contains
padded SHA1 template hashes if SHA-256 was unavailable at IMA init.

I would argue that it's likely that no existing userspace tool is relying
on this fallback logic -- they either wouldn't consume the hash value from
the measurement list directly but recreate it by themselves, as is required
for verification against PCRs, or, if they did, they would somehow assume a
hash algorithm and expect the hashes in the measurement list to be of that
type. If of the latter kind, this could even lead to hard to debug
verification failures. For example, from looking at keylime's current
code, the verifier logic seems to assume that the template hashes found
in the provided measurement list are of the configured 'ima_log_hash_alg'
type. In particular, it does not check against padded SHA1 upon
mismatch.

That being said, there's also another dimension: currently IMA has a
hard requirement on SHA-1 and subsequent patches in this series will
attempt to get rid of that. If SHA-1 is not available at IMA init though,
it would also mean that padded SHA-1 values cannot get filled in as a
fallback for other unsupported algorithms. Substituting something like
hard coded all-zeroes or all-ones would be dangerous, because some
application or user scripts could perhaps (ab)use the template hashes from
the exported measurement lists for some kind of fingerprinting scheme or
so.

In conclusion, I think it's best to not create the
runtime_measurements_<hash-algo> sysfs files for hash algorithms not
supported by the kernel. That way, applications expecting a certain
hash algorithm for the measurement list and which are not able to handle
the padded-SHA1 fallback scheme would fail with a clear indication on what
the problem is. Furthermore, as digests for unsupported banks are not
getting exposed to userspace anymore, we'll have all flexibility to
set it to any value internally, including all-ones as will be needed in
a subsequent patch when addressing PCR extend for unsupported banks.

So, do not create runtime_measurements_<hash-algo> sysfs files for
unsupported hash algorithms. Likewise for their ascii counterparts.

Note that at this point, SHA-1 is still mandatory, and thus,
runtime_measurements_sha1 as well as the "runtime_measurements" will
remain there, even though the code has provisions already to skip their
creation as well in case SHA-1 was unavailable.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/ima_fs.c | 35 +++++++++++++++++++++------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e4a79a9b2d58..a8df2fe5f4cb 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -454,6 +454,9 @@ static int __init create_securityfs_measurement_lists(void)
 		return -ENOMEM;
 
 	for (i = 0; i < securityfs_measurement_list_count; i++) {
+		if (!ima_algo_array[i].tfm)
+			continue;
+
 		algo = ima_algo_array[i].algo;
 
 		sprintf(file_name, "ascii_runtime_measurements_%s",
@@ -573,20 +576,26 @@ int __init ima_fs_init(void)
 	if (ret != 0)
 		goto out;
 
-	binary_runtime_measurements =
-	    securityfs_create_symlink("binary_runtime_measurements", ima_dir,
-				      "binary_runtime_measurements_sha1", NULL);
-	if (IS_ERR(binary_runtime_measurements)) {
-		ret = PTR_ERR(binary_runtime_measurements);
-		goto out;
-	}
+	if (ima_algo_array[ima_sha1_idx].tfm) {
+		binary_runtime_measurements =
+		    securityfs_create_symlink("binary_runtime_measurements",
+					      ima_dir,
+					      "binary_runtime_measurements_sha1",
+					      NULL);
+		if (IS_ERR(binary_runtime_measurements)) {
+			ret = PTR_ERR(binary_runtime_measurements);
+			goto out;
+		}
 
-	ascii_runtime_measurements =
-	    securityfs_create_symlink("ascii_runtime_measurements", ima_dir,
-				      "ascii_runtime_measurements_sha1", NULL);
-	if (IS_ERR(ascii_runtime_measurements)) {
-		ret = PTR_ERR(ascii_runtime_measurements);
-		goto out;
+		ascii_runtime_measurements =
+		    securityfs_create_symlink("ascii_runtime_measurements",
+					      ima_dir,
+					      "ascii_runtime_measurements_sha1",
+					      NULL);
+		if (IS_ERR(ascii_runtime_measurements)) {
+			ret = PTR_ERR(ascii_runtime_measurements);
+			goto out;
+		}
 	}
 
 	runtime_measurements_count =
-- 
2.47.1

[RFC PATCH v1 2/7] ima: always create runtime_measurements sysfs file for ima_hash

[Nicolai Stange]

runtime_measurements_<hash-algo> sysfs files are getting created for
each PCR bank + for SHA-1.

Now that runtime_measurements_<hash-algo> sysfs file creation is being
skipped for unsupported hash algorithms, it will become possible that no
such file would be provided at all once SHA-1 is made optional in a
later patch.

Always create the file for the 'ima_hash' algorithm, even if it's not
associated with any of the PCR banks. As IMA initialization will
continue to fail if the ima_hash algorithm is not available to the
kernel, this guarantees that at least one such file will always be
there.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/ima_fs.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index a8df2fe5f4cb..f030ff7f56da 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -436,10 +436,8 @@ static int __init create_securityfs_measurement_lists(void)
 	u16 algo;
 	int i;
 
-	securityfs_measurement_list_count = NR_BANKS(ima_tpm_chip);
-
-	if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip))
-		securityfs_measurement_list_count++;
+	securityfs_measurement_list_count =
+		NR_BANKS(ima_tpm_chip) + ima_extra_slots;
 
 	ascii_securityfs_measurement_lists =
 	    kcalloc(securityfs_measurement_list_count, sizeof(struct dentry *),
-- 
2.47.1

[RFC PATCH v1 3/7] ima: move INVALID_PCR() to ima.h

[Nicolai Stange]

Make the INVALID_PCR() #define available to other compilation units
by moving it from ima_policy.c to ima.h and renaming it to
IMA_INVALID_PCR() in the course.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/ima.h        | 4 ++++
 security/integrity/ima/ima_policy.c | 5 +----
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a4f284bd846c..1158a7b8bf6b 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -198,6 +198,10 @@ 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))
+
+
 extern struct lsm_blob_sizes ima_blob_sizes;
 
 static inline struct ima_iint_cache *
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 128fab897930..d9e4210ea814 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -48,9 +48,6 @@
 #define HASH		0x0100
 #define DONT_HASH	0x0200
 
-#define INVALID_PCR(a) (((a) < 0) || \
-	(a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
-
 int ima_policy_flag;
 static int temp_ima_appraise;
 static int build_ima_appraise __ro_after_init;
@@ -1855,7 +1852,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			ima_log_string(ab, "pcr", args[0].from);
 
 			result = kstrtoint(args[0].from, 10, &entry->pcr);
-			if (result || INVALID_PCR(entry->pcr))
+			if (result || IMA_INVALID_PCR(entry->pcr))
 				result = -EINVAL;
 			else
 				entry->flags |= IMA_PCR;
-- 
2.47.1

[RFC PATCH v1 4/7] ima: track the set of PCRs ever extended

[Nicolai Stange]

A subsequent patch will make IMA to invalidate PCR banks associated with
unsupported hash algorithms 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.

Make ima_restore_measurement_list() to restore the ima_extended_pcrs_mask
from the measurement list in order to maintain it across kexecs.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/ima.h          | 8 ++++++--
 security/integrity/ima/ima_queue.c    | 6 ++++++
 security/integrity/ima/ima_template.c | 7 +++++++
 3 files changed, 19 insertions(+), 2 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 83d53824aa98..f00ba2222c34 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)
@@ -152,6 +157,7 @@ static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
 	result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
 	if (result != 0)
 		pr_err("Error Communicating to TPM chip, result: %d\n", result);
+	ima_extended_pcrs_mask |= BIT(pcr);
 	return result;
 }
 
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 04c49f05cb74..55d335097d85 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -525,12 +525,19 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 			}
 		}
 
+		if (IMA_INVALID_PCR(entry->pcr)) {
+			pr_err("invalid measurement PCR index");
+			ret = -EINVAL;
+			break;
+		}
+
 		entry->pcr = !ima_canonical_fmt ? *(u32 *)(hdr[HDR_PCR].data) :
 			     le32_to_cpu(*(__le32 *)(hdr[HDR_PCR].data));
 		ret = ima_restore_measurement_entry(entry);
 		if (ret < 0)
 			break;
 
+		ima_extended_pcrs_mask |= BIT(entry->pcr);
 	}
 	return ret;
 }
-- 
2.47.1

[RFC PATCH v1 5/7] tpm: enable bank selection for PCR extend

[Nicolai Stange]

The existing tpm_pcr_extend() extends all of a PCR's allocated banks with
the corresponding digest from the provided digests[] argument.

An upcoming code change to IMA will introduce the need to skip over those
banks it does not have a hash algorithm implementation available for.

Introduce tpm_pcr_extend_sel() to support this.

tpm_pcr_extend_sel() also expects a digests[] array, always being the
number of allocated PCR banks in size, just as it's the case for the
existing tpm_pcr_extend(). In addition to that however, it takes a
'banks_skip_mask', and will skip the extension of any bank having its
corresponding bit set there.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 drivers/char/tpm/tpm-interface.c | 29 +++++++++++++++++++++++++++--
 drivers/char/tpm/tpm.h           |  3 ++-
 drivers/char/tpm/tpm2-cmd.c      | 29 +++++++++++++++++++++++++++--
 include/linux/tpm.h              |  3 +++
 4 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index b1daa0d7b341..2bab251034b5 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -314,6 +314,26 @@ EXPORT_SYMBOL_GPL(tpm_pcr_read);
  */
 int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		   struct tpm_digest *digests)
+{
+	return tpm_pcr_extend_sel(chip, pcr_idx, digests, 0);
+}
+EXPORT_SYMBOL_GPL(tpm_pcr_extend);
+
+/**
+ * tpm_pcr_extend_sel - extend a PCR value into selected banks.
+ * @chip:	a &struct tpm_chip instance, %NULL for the default chip
+ * @pcr_idx:	the PCR to be retrieved
+ * @digests:	array of tpm_digest structures used to extend PCRs
+ * @banks_skip_mask:	pcr banks to skip
+ *
+ * Note: callers must pass a digest for every allocated PCR bank, in the same
+ * order of the banks in chip->allocated_banks.
+ *
+ * Return: same as with tpm_transmit_cmd()
+ */
+int tpm_pcr_extend_sel(struct tpm_chip *chip, u32 pcr_idx,
+		       struct tpm_digest *digests,
+		       unsigned long banks_skip_mask)
 {
 	int rc;
 	int i;
@@ -330,7 +350,13 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	}
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
-		rc = tpm2_pcr_extend(chip, pcr_idx, digests);
+		rc = tpm2_pcr_extend(chip, pcr_idx, digests, 0);
+		goto out;
+	}
+
+	/* There's only one SHA1 bank with TPM 1. */
+	if (banks_skip_mask & 1) {
+		rc = 0;
 		goto out;
 	}
 
@@ -341,7 +367,6 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	tpm_put_ops(chip);
 	return rc;
 }
-EXPORT_SYMBOL_GPL(tpm_pcr_extend);
 
 int tpm_auto_startup(struct tpm_chip *chip)
 {
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 7bb87fa5f7a1..f4ed49cb4101 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -291,7 +291,8 @@ int tpm2_get_timeouts(struct tpm_chip *chip);
 int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 		  struct tpm_digest *digest, u16 *digest_size_ptr);
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
-		    struct tpm_digest *digests);
+		    struct tpm_digest *digests,
+		    unsigned long banks_skip_mask);
 int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max);
 ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
 			u32 *value, const char *desc);
diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index dfdcbd009720..23ded8ea47dc 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -226,16 +226,34 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
  * @chip:	TPM chip to use.
  * @pcr_idx:	index of the PCR.
  * @digests:	list of pcr banks and corresponding digest values to extend.
+ * @banks_skip_mask:	pcr banks to skip
  *
  * Return: Same as with tpm_transmit_cmd.
  */
 int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
-		    struct tpm_digest *digests)
+		    struct tpm_digest *digests,
+		    unsigned long banks_skip_mask)
 {
 	struct tpm_buf buf;
+	unsigned long skip_mask;
+	u32 banks_count;
 	int rc;
 	int i;
 
+	banks_count = 0;
+	skip_mask = banks_skip_mask;
+	for (i = 0; i < chip->nr_allocated_banks; i++) {
+		const bool skip_bank = skip_mask & 1;
+
+		skip_mask >>= 1;
+		if (skip_bank)
+			continue;
+		banks_count++;
+	}
+
+	if (banks_count == 0)
+		return 0;
+
 	if (!disable_pcr_integrity) {
 		rc = tpm2_start_auth_session(chip);
 		if (rc)
@@ -257,9 +275,16 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 		tpm_buf_append_auth(chip, &buf, 0, NULL, 0);
 	}
 
-	tpm_buf_append_u32(&buf, chip->nr_allocated_banks);
+	tpm_buf_append_u32(&buf, banks_count);
 
+	skip_mask = banks_skip_mask;
 	for (i = 0; i < chip->nr_allocated_banks; i++) {
+		const bool skip_bank = skip_mask & 1;
+
+		skip_mask >>= 1;
+		if (skip_bank)
+			continue;
+
 		tpm_buf_append_u16(&buf, digests[i].alg_id);
 		tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest,
 			       chip->allocated_banks[i].digest_size);
diff --git a/include/linux/tpm.h b/include/linux/tpm.h
index 20a40ade8030..7587eecc82fd 100644
--- a/include/linux/tpm.h
+++ b/include/linux/tpm.h
@@ -447,6 +447,9 @@ extern int tpm_pcr_read(struct tpm_chip *chip, u32 pcr_idx,
 			struct tpm_digest *digest);
 extern int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 			  struct tpm_digest *digests);
+extern int tpm_pcr_extend_sel(struct tpm_chip *chip, u32 pcr_idx,
+			      struct tpm_digest *digests,
+			      unsigned long banks_skip_mask);
 extern int tpm_get_random(struct tpm_chip *chip, u8 *data, size_t max);
 extern struct tpm_chip *tpm_default_chip(void);
 void tpm2_flush_context(struct tpm_chip *chip, u32 handle);
-- 
2.47.1

[RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Nicolai Stange]

Normally IMA would extend a template hash of each bank's associated
algorithm into a PCR. However, if a bank's hash algorithm is unavailable
to the kernel at IMA init time, it would fallback to extending padded
SHA1 hashes instead.

That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
SHA1 template hashes into a PCR's SHA-256 bank.

The ima_measurement command (marked as experimental) from ima-evm-utils
would accordingly try both variants when attempting to verify a measurement
list against PCRs. keylime OTOH doesn't seem to -- it expects the template
hash type to match the PCR bank algorithm. I would argue that for the
latter case, the fallback scheme could potentially cause hard to debug
verification failures.

There's another problem with the fallback scheme: right now, SHA-1
availability is a hard requirement for IMA, and it would be good for a
number of reasons to get rid of that. However, if SHA-1 is not available to
the kernel, it can hardly provide padded SHA-1 template hashes for PCR
banks with unsupported algos.

There are several more or less reasonable alternatives possible, among
them are:
a.) Instead of padded SHA-1, use padded/truncated ima_hash template
    hashes.
b.) Record every event as a violation, i.e. extend unsupported banks
    with 0xffs.
c.) Don't extend unsupported banks at all.
d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
    use.

a.) would make verification from tools like ima_measurement nearly
    impossible, as it would have to guess or somehow determine ima_hash.
b.) would still put an significant and unnecessary burden on tools like
    ima_measurement, because it would then have to exercise three
    possible variants on the measurement list:
    - the template hash matches the bank algorithm,
    - the template hash is padded SHA-1,
    - the template hash is all-ones.
c.) is a security risk, because the bank would validate an empty
    measurement list.

AFAICS, d.) is the best option to proceed, as it allows for determining
from the PCR bank value in O(1) whether the bank had been maintained by
IMA or not and also, it would not validate any measurement list (except
one with a single violation entry at the head).

So implement d.). As it potentially breaks existing userspace, i.e.
the current implementation of ima_measurement, put it behind a Kconfig
option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
behavior of extending with padded SHA-1 is retained. Otherwise the new
scheme to invalidate unsupported PCR banks once upon their first extension
from IMA is implemented instead. As ima_measurement is marked as
experimental and I find it unlikely that other existing tools depend on
the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
Kconfig option default to "n".

For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
- make ima_calc_field_array_hash() to fill the digests corresponding to
  banks with unsupported hash algorithms with 0xffs,
- make ima_pcr_extend() to extend these into the unsupported PCR banks only
  upon the PCR's first usage, skip them on subsequent updates and
- let ima_init_ima_crypto() help it with that by populating the new
  ima_unsupported_tpm_banks_mask with one bit set for each bank with
  an unavailable hash algorithm at init.

[1] https://github.com/linux-integrity/ima-evm-utils

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/Kconfig      | 14 ++++++++++++++
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_crypto.c | 27 +++++++++++++++++++++++++--
 security/integrity/ima/ima_queue.c  | 20 +++++++++++++++++++-
 4 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 475c32615006..d6ba392c0b37 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -122,6 +122,20 @@ config IMA_DEFAULT_HASH
 	default "wp512" if IMA_DEFAULT_HASH_WP512
 	default "sm3" if IMA_DEFAULT_HASH_SM3
 
+config IMA_COMPAT_FALLBACK_TPM_EXTEND
+	bool
+	default n
+	help
+	  In case a TPM PCR hash algorithm is not supported by the kernel,
+	  retain the old behaviour to extend the bank with padded SHA1 template
+	  digests.
+
+	  If Y, IMA will be unavailable when SHA1 is missing from the kernel.
+	  If N, existing tools may fail to verify IMA measurement lists against
+	  TPM PCR banks corresponding to hashes not supported by the kernel.
+
+	  If unsure, say N.
+
 config IMA_WRITE_POLICY
 	bool "Enable multiple writes to the IMA policy"
 	default n
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f99b1f81b35c..58e9a81b3f96 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -62,6 +62,7 @@ extern int ima_sha1_idx __ro_after_init;
 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_unsupported_tpm_banks_mask __ro_after_init;
 
 extern unsigned long ima_extended_pcrs_mask;
 
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 6f5696d999d0..118ea15d737b 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -67,6 +67,8 @@ int ima_extra_slots __ro_after_init;
 
 struct ima_algo_desc *ima_algo_array __ro_after_init;
 
+unsigned long ima_unsupported_tpm_banks_mask __ro_after_init;
+
 static int __init ima_init_ima_crypto(void)
 {
 	long rc;
@@ -150,8 +152,10 @@ int __init ima_init_crypto(void)
 		ima_algo_array[i].algo = algo;
 
 		/* unknown TPM algorithm */
-		if (algo == HASH_ALGO__LAST)
+		if (algo == HASH_ALGO__LAST) {
+			ima_unsupported_tpm_banks_mask |= BIT(i);
 			continue;
+		}
 
 		if (algo == ima_hash_algo) {
 			ima_algo_array[i].tfm = ima_shash_tfm;
@@ -167,6 +171,7 @@ int __init ima_init_crypto(void)
 			}
 
 			ima_algo_array[i].tfm = NULL;
+			ima_unsupported_tpm_banks_mask |= BIT(i);
 		}
 	}
 
@@ -625,26 +630,44 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	u16 alg_id;
 	int rc, i;
 
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
 	rc = ima_calc_field_array_hash_tfm(field_data, entry, ima_sha1_idx);
 	if (rc)
 		return rc;
 
 	entry->digests[ima_sha1_idx].alg_id = TPM_ALG_SHA1;
+#endif
 
 	for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
 		if (i == ima_sha1_idx)
 			continue;
+#endif
 
 		if (i < NR_BANKS(ima_tpm_chip)) {
 			alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
 			entry->digests[i].alg_id = alg_id;
 		}
 
-		/* for unmapped TPM algorithms digest is still a padded SHA1 */
+		/*
+		 * For unmapped TPM algorithms, the digest is still a
+		 * padded SHA1 if backwards-compatibility fallback PCR
+		 * extension is enabled.  Otherwise fill with
+		 * 0xffs. This is the value to invalidate unsupported
+		 * PCR banks with once at first PCR use. Also, a
+		 * non-all-zeroes value serves as an indicator to
+		 * kexec measurement restoration that the entry is not
+		 * a violation and all its template digests need to
+		 * get recomputed.
+		 */
 		if (!ima_algo_array[i].tfm) {
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
 			memcpy(entry->digests[i].digest,
 			       entry->digests[ima_sha1_idx].digest,
 			       TPM_DIGEST_SIZE);
+#else
+			memset(entry->digests[i].digest, 0xff, TPM_DIGEST_SIZE);
+#endif
 			continue;
 		}
 
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index f00ba2222c34..4db6c4be58fc 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -150,11 +150,27 @@ unsigned long ima_get_binary_runtime_size(void)
 static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
 {
 	int result = 0;
+	unsigned long tpm_banks_skip_mask;
 
 	if (!ima_tpm_chip)
 		return result;
 
-	result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
+#if !IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
+	tpm_banks_skip_mask = ima_unsupported_tpm_banks_mask;
+	if (!(ima_extended_pcrs_mask & BIT(pcr))) {
+		/*
+		 * Invalidate unsupported banks once upon a PCR's
+		 * first usage. Note that the digests[] entries for
+		 * unsupported algorithms have been filled with 0xffs.
+		 */
+		tpm_banks_skip_mask = 0;
+	}
+#else
+	tpm_banks_skip_mask = 0;
+#endif
+
+	result = tpm_pcr_extend_sel(ima_tpm_chip, pcr, digests_arg,
+				    tpm_banks_skip_mask);
 	if (result != 0)
 		pr_err("Error Communicating to TPM chip, result: %d\n", result);
 	ima_extended_pcrs_mask |= BIT(pcr);
@@ -280,9 +296,11 @@ int __init ima_init_digests(void)
 		digest_size = ima_tpm_chip->allocated_banks[i].digest_size;
 		crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
 
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
 		/* for unmapped TPM algorithms digest is still a padded SHA1 */
 		if (crypto_id == HASH_ALGO__LAST)
 			digest_size = SHA1_DIGEST_SIZE;
+#endif
 
 		memset(digests[i].digest, 0xff, digest_size);
 	}
-- 
2.47.1

[RFC PATCH v1 7/7] ima: make SHA1 non-mandatory

[Nicolai Stange]

For CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND=n, SHA-1 is not a hard
requirement anymore. Make ima_init_crypto() continue on SHA-1
instantiation errors.

Note that the configure ima_hash must still be available. If that
happened to be set to SHA-1 and SHA-1 was missing, then IMA would
still fail to initialize.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 security/integrity/ima/ima_crypto.c | 60 ++++++++++++++---------------
 1 file changed, 28 insertions(+), 32 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 118ea15d737b..f68435f2679f 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -147,56 +147,51 @@ int __init ima_init_crypto(void)
 		goto out;
 	}
 
+	ima_algo_array[ima_hash_algo_idx].tfm = ima_shash_tfm;
+	ima_algo_array[ima_hash_algo_idx].algo = ima_hash_algo;
+
+	if (ima_hash_algo != HASH_ALGO_SHA1) {
+		ima_algo_array[ima_sha1_idx].tfm =
+			ima_alloc_tfm(HASH_ALGO_SHA1);
+		if (IS_ERR(ima_algo_array[ima_sha1_idx].tfm)) {
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
+			/*
+			 * For backwards compatible fallback PCR
+			 * extension, SHA1 is the fallback for missing
+			 * algos.
+			 */
+			rc = PTR_ERR(ima_algo_array[ima_sha1_idx].tfm);
+			goto out_array;
+#endif
+			ima_algo_array[ima_sha1_idx].tfm = NULL;
+			ima_unsupported_tpm_banks_mask |= BIT(ima_sha1_idx);
+		}
+		ima_algo_array[ima_sha1_idx].algo = HASH_ALGO_SHA1;
+	}
+
 	for (i = 0; i < NR_BANKS(ima_tpm_chip); i++) {
 		algo = ima_tpm_chip->allocated_banks[i].crypto_id;
 		ima_algo_array[i].algo = algo;
 
+		/* Initialized separately above. */
+		if (i == ima_hash_algo_idx || i == ima_sha1_idx)
+			continue;
+
 		/* unknown TPM algorithm */
 		if (algo == HASH_ALGO__LAST) {
 			ima_unsupported_tpm_banks_mask |= BIT(i);
 			continue;
 		}
 
-		if (algo == ima_hash_algo) {
-			ima_algo_array[i].tfm = ima_shash_tfm;
-			continue;
-		}
-
 		ima_algo_array[i].tfm = ima_alloc_tfm(algo);
 		if (IS_ERR(ima_algo_array[i].tfm)) {
-			if (algo == HASH_ALGO_SHA1) {
-				rc = PTR_ERR(ima_algo_array[i].tfm);
-				ima_algo_array[i].tfm = NULL;
-				goto out_array;
-			}
-
 			ima_algo_array[i].tfm = NULL;
 			ima_unsupported_tpm_banks_mask |= BIT(i);
 		}
 	}
 
-	if (ima_sha1_idx >= NR_BANKS(ima_tpm_chip)) {
-		if (ima_hash_algo == HASH_ALGO_SHA1) {
-			ima_algo_array[ima_sha1_idx].tfm = ima_shash_tfm;
-		} else {
-			ima_algo_array[ima_sha1_idx].tfm =
-						ima_alloc_tfm(HASH_ALGO_SHA1);
-			if (IS_ERR(ima_algo_array[ima_sha1_idx].tfm)) {
-				rc = PTR_ERR(ima_algo_array[ima_sha1_idx].tfm);
-				goto out_array;
-			}
-		}
-
-		ima_algo_array[ima_sha1_idx].algo = HASH_ALGO_SHA1;
-	}
-
-	if (ima_hash_algo_idx >= NR_BANKS(ima_tpm_chip) &&
-	    ima_hash_algo_idx != ima_sha1_idx) {
-		ima_algo_array[ima_hash_algo_idx].tfm = ima_shash_tfm;
-		ima_algo_array[ima_hash_algo_idx].algo = ima_hash_algo;
-	}
-
 	return 0;
+#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
 out_array:
 	for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
 		if (!ima_algo_array[i].tfm ||
@@ -206,6 +201,7 @@ int __init ima_init_crypto(void)
 		crypto_free_shash(ima_algo_array[i].tfm);
 	}
 	kfree(ima_algo_array);
+#endif
 out:
 	crypto_free_shash(ima_shash_tfm);
 	return rc;
-- 
2.47.1

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Mimi Zohar]

On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
> Normally IMA would extend a template hash of each bank's associated
> algorithm into a PCR. However, if a bank's hash algorithm is unavailable
> to the kernel at IMA init time, it would fallback to extending padded
> SHA1 hashes instead.
> 
> That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
> SHA1 template hashes into a PCR's SHA-256 bank.
> 
> The ima_measurement command (marked as experimental) from ima-evm-utils
> would accordingly try both variants when attempting to verify a measurement
> list against PCRs. keylime OTOH doesn't seem to -- it expects the template
> hash type to match the PCR bank algorithm. I would argue that for the
> latter case, the fallback scheme could potentially cause hard to debug
> verification failures.
> 
> There's another problem with the fallback scheme: right now, SHA-1
> availability is a hard requirement for IMA, and it would be good for a
> number of reasons to get rid of that. However, if SHA-1 is not available to
> the kernel, it can hardly provide padded SHA-1 template hashes for PCR
> banks with unsupported algos.
> 
> There are several more or less reasonable alternatives possible, among
> them are:
> a.) Instead of padded SHA-1, use padded/truncated ima_hash template
>     hashes.
> b.) Record every event as a violation, i.e. extend unsupported banks
>     with 0xffs.
> c.) Don't extend unsupported banks at all.
> d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
>     use.
> 
> a.) would make verification from tools like ima_measurement nearly
>     impossible, as it would have to guess or somehow determine ima_hash.
> b.) would still put an significant and unnecessary burden on tools like
>     ima_measurement, because it would then have to exercise three
>     possible variants on the measurement list:
>     - the template hash matches the bank algorithm,
>     - the template hash is padded SHA-1,
>     - the template hash is all-ones.
> c.) is a security risk, because the bank would validate an empty
>     measurement list.
> 
> AFAICS, d.) is the best option to proceed, as it allows for determining
> from the PCR bank value in O(1) whether the bank had been maintained by
> IMA or not and also, it would not validate any measurement list (except
> one with a single violation entry at the head).

Hi Nicolai,

What a pleasure reviewing your patch set.  Nicely organized.  Well written patch
descriptions.

Currently with the SHA1 hash algorithm, whether it is being extended into the
TPM or not, the measurement list is complete.  Relying on the ima_hash in the
current kernel and the subsequent kexec'ed kernel should be fine, assuming if
they're different hash algorithms both TPM banks are enabled.  Otherwise, the
measurement lists will be incomplete.

This patch set introduces a new definition of integrity violation. Previously it
was limited to open-writers and ToMToU integrity violations.  Now it could also
mean no kernel hash algorithm available.  Unfortunately some attestation
services simply ignore integrity violations.

Mimi

> 
> So implement d.). As it potentially breaks existing userspace, i.e.
> the current implementation of ima_measurement, put it behind a Kconfig
> option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
> behavior of extending with padded SHA-1 is retained. Otherwise the new
> scheme to invalidate unsupported PCR banks once upon their first extension
> from IMA is implemented instead. As ima_measurement is marked as
> experimental and I find it unlikely that other existing tools depend on
> the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
> Kconfig option default to "n".
> 
> For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
> - make ima_calc_field_array_hash() to fill the digests corresponding to
>   banks with unsupported hash algorithms with 0xffs,
> - make ima_pcr_extend() to extend these into the unsupported PCR banks only
>   upon the PCR's first usage, skip them on subsequent updates and
> - let ima_init_ima_crypto() help it with that by populating the new
>   ima_unsupported_tpm_banks_mask with one bit set for each bank with
>   an unavailable hash algorithm at init.
> 
> [1] https://github.com/linux-integrity/ima-evm-utils
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>
> ---
>  security/integrity/ima/Kconfig      | 14 ++++++++++++++
>  security/integrity/ima/ima.h        |  1 +
>  security/integrity/ima/ima_crypto.c | 27 +++++++++++++++++++++++++--
>  security/integrity/ima/ima_queue.c  | 20 +++++++++++++++++++-
>  4 files changed, 59 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 475c32615006..d6ba392c0b37 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -122,6 +122,20 @@ config IMA_DEFAULT_HASH
>  	default "wp512" if IMA_DEFAULT_HASH_WP512
>  	default "sm3" if IMA_DEFAULT_HASH_SM3
>  
> +config IMA_COMPAT_FALLBACK_TPM_EXTEND
> +	bool
> +	default n
> +	help
> +	  In case a TPM PCR hash algorithm is not supported by the kernel,
> +	  retain the old behaviour to extend the bank with padded SHA1 template
> +	  digests.
> +
> +	  If Y, IMA will be unavailable when SHA1 is missing from the kernel.
> +	  If N, existing tools may fail to verify IMA measurement lists against
> +	  TPM PCR banks corresponding to hashes not supported by the kernel.
> +
> +	  If unsure, say N.
> +
>  config IMA_WRITE_POLICY
>  	bool "Enable multiple writes to the IMA policy"
>  	default n
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index f99b1f81b35c..58e9a81b3f96 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -62,6 +62,7 @@ extern int ima_sha1_idx __ro_after_init;
>  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_unsupported_tpm_banks_mask __ro_after_init;
>  
>  extern unsigned long ima_extended_pcrs_mask;
>  
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 6f5696d999d0..118ea15d737b 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -67,6 +67,8 @@ int ima_extra_slots __ro_after_init;
>  
>  struct ima_algo_desc *ima_algo_array __ro_after_init;
>  
> +unsigned long ima_unsupported_tpm_banks_mask __ro_after_init;
> +
>  static int __init ima_init_ima_crypto(void)
>  {
>  	long rc;
> @@ -150,8 +152,10 @@ int __init ima_init_crypto(void)
>  		ima_algo_array[i].algo = algo;
>  
>  		/* unknown TPM algorithm */
> -		if (algo == HASH_ALGO__LAST)
> +		if (algo == HASH_ALGO__LAST) {
> +			ima_unsupported_tpm_banks_mask |= BIT(i);
>  			continue;
> +		}
>  
>  		if (algo == ima_hash_algo) {
>  			ima_algo_array[i].tfm = ima_shash_tfm;
> @@ -167,6 +171,7 @@ int __init ima_init_crypto(void)
>  			}
>  
>  			ima_algo_array[i].tfm = NULL;
> +			ima_unsupported_tpm_banks_mask |= BIT(i);
>  		}
>  	}
>  
> @@ -625,26 +630,44 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
>  	u16 alg_id;
>  	int rc, i;
>  
> +#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
>  	rc = ima_calc_field_array_hash_tfm(field_data, entry, ima_sha1_idx);
>  	if (rc)
>  		return rc;
>  
>  	entry->digests[ima_sha1_idx].alg_id = TPM_ALG_SHA1;
> +#endif
>  
>  	for (i = 0; i < NR_BANKS(ima_tpm_chip) + ima_extra_slots; i++) {
> +#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
>  		if (i == ima_sha1_idx)
>  			continue;
> +#endif
>  
>  		if (i < NR_BANKS(ima_tpm_chip)) {
>  			alg_id = ima_tpm_chip->allocated_banks[i].alg_id;
>  			entry->digests[i].alg_id = alg_id;
>  		}
>  
> -		/* for unmapped TPM algorithms digest is still a padded SHA1 */
> +		/*
> +		 * For unmapped TPM algorithms, the digest is still a
> +		 * padded SHA1 if backwards-compatibility fallback PCR
> +		 * extension is enabled.  Otherwise fill with
> +		 * 0xffs. This is the value to invalidate unsupported
> +		 * PCR banks with once at first PCR use. Also, a
> +		 * non-all-zeroes value serves as an indicator to
> +		 * kexec measurement restoration that the entry is not
> +		 * a violation and all its template digests need to
> +		 * get recomputed.
> +		 */
>  		if (!ima_algo_array[i].tfm) {
> +#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
>  			memcpy(entry->digests[i].digest,
>  			       entry->digests[ima_sha1_idx].digest,
>  			       TPM_DIGEST_SIZE);
> +#else
> +			memset(entry->digests[i].digest, 0xff, TPM_DIGEST_SIZE);
> +#endif
>  			continue;
>  		}
>  
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index f00ba2222c34..4db6c4be58fc 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -150,11 +150,27 @@ unsigned long ima_get_binary_runtime_size(void)
>  static int ima_pcr_extend(struct tpm_digest *digests_arg, int pcr)
>  {
>  	int result = 0;
> +	unsigned long tpm_banks_skip_mask;
>  
>  	if (!ima_tpm_chip)
>  		return result;
>  
> -	result = tpm_pcr_extend(ima_tpm_chip, pcr, digests_arg);
> +#if !IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
> +	tpm_banks_skip_mask = ima_unsupported_tpm_banks_mask;
> +	if (!(ima_extended_pcrs_mask & BIT(pcr))) {
> +		/*
> +		 * Invalidate unsupported banks once upon a PCR's
> +		 * first usage. Note that the digests[] entries for
> +		 * unsupported algorithms have been filled with 0xffs.
> +		 */
> +		tpm_banks_skip_mask = 0;
> +	}
> +#else
> +	tpm_banks_skip_mask = 0;
> +#endif
> +
> +	result = tpm_pcr_extend_sel(ima_tpm_chip, pcr, digests_arg,
> +				    tpm_banks_skip_mask);
>  	if (result != 0)
>  		pr_err("Error Communicating to TPM chip, result: %d\n", result);
>  	ima_extended_pcrs_mask |= BIT(pcr);
> @@ -280,9 +296,11 @@ int __init ima_init_digests(void)
>  		digest_size = ima_tpm_chip->allocated_banks[i].digest_size;
>  		crypto_id = ima_tpm_chip->allocated_banks[i].crypto_id;
>  
> +#if IS_ENABLED(CONFIG_IMA_COMPAT_FALLBACK_TPM_EXTEND)
>  		/* for unmapped TPM algorithms digest is still a padded SHA1 */
>  		if (crypto_id == HASH_ALGO__LAST)
>  			digest_size = SHA1_DIGEST_SIZE;
> +#endif
>  
>  		memset(digests[i].digest, 0xff, digest_size);
>  	}

Re: [RFC PATCH v1 3/7] ima: move INVALID_PCR() to ima.h

[Mimi Zohar]

On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
> Make the INVALID_PCR() #define available to other compilation units
> by moving it from ima_policy.c to ima.h and renaming it to
> IMA_INVALID_PCR() in the course.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>

Restoring the IMA measurement list doesn't involve extending the TPM.  The hash
specific measurements have already been extended into the respective TPM banks.
Is this and the subsequent patch necessary?

Mimi


> ---
>  security/integrity/ima/ima.h        | 4 ++++
>  security/integrity/ima/ima_policy.c | 5 +----
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index a4f284bd846c..1158a7b8bf6b 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -198,6 +198,10 @@ 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))
> +
> +
>  extern struct lsm_blob_sizes ima_blob_sizes;
>  
>  static inline struct ima_iint_cache *
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 128fab897930..d9e4210ea814 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -48,9 +48,6 @@
>  #define HASH		0x0100
>  #define DONT_HASH	0x0200
>  
> -#define INVALID_PCR(a) (((a) < 0) || \
> -	(a) >= (sizeof_field(struct ima_iint_cache, measured_pcrs) * 8))
> -
>  int ima_policy_flag;
>  static int temp_ima_appraise;
>  static int build_ima_appraise __ro_after_init;
> @@ -1855,7 +1852,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			ima_log_string(ab, "pcr", args[0].from);
>  
>  			result = kstrtoint(args[0].from, 10, &entry->pcr);
> -			if (result || INVALID_PCR(entry->pcr))
> +			if (result || IMA_INVALID_PCR(entry->pcr))
>  				result = -EINVAL;
>  			else
>  				entry->flags |= IMA_PCR;

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Nicolai Stange]

Mimi Zohar <zohar@linux.ibm.com> writes:

> On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
>> Normally IMA would extend a template hash of each bank's associated
>> algorithm into a PCR. However, if a bank's hash algorithm is unavailable
>> to the kernel at IMA init time, it would fallback to extending padded
>> SHA1 hashes instead.
>> 
>> That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
>> SHA1 template hashes into a PCR's SHA-256 bank.
>> 
>> The ima_measurement command (marked as experimental) from ima-evm-utils
>> would accordingly try both variants when attempting to verify a measurement
>> list against PCRs. keylime OTOH doesn't seem to -- it expects the template
>> hash type to match the PCR bank algorithm. I would argue that for the
>> latter case, the fallback scheme could potentially cause hard to debug
>> verification failures.
>> 
>> There's another problem with the fallback scheme: right now, SHA-1
>> availability is a hard requirement for IMA, and it would be good for a
>> number of reasons to get rid of that. However, if SHA-1 is not available to
>> the kernel, it can hardly provide padded SHA-1 template hashes for PCR
>> banks with unsupported algos.
>> 
>> There are several more or less reasonable alternatives possible, among
>> them are:
>> a.) Instead of padded SHA-1, use padded/truncated ima_hash template
>>     hashes.
>> b.) Record every event as a violation, i.e. extend unsupported banks
>>     with 0xffs.
>> c.) Don't extend unsupported banks at all.
>> d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
>>     use.
>> 
>> a.) would make verification from tools like ima_measurement nearly
>>     impossible, as it would have to guess or somehow determine ima_hash.
>> b.) would still put an significant and unnecessary burden on tools like
>>     ima_measurement, because it would then have to exercise three
>>     possible variants on the measurement list:
>>     - the template hash matches the bank algorithm,
>>     - the template hash is padded SHA-1,
>>     - the template hash is all-ones.
>> c.) is a security risk, because the bank would validate an empty
>>     measurement list.
>> 
>> AFAICS, d.) is the best option to proceed, as it allows for determining
>> from the PCR bank value in O(1) whether the bank had been maintained by
>> IMA or not and also, it would not validate any measurement list (except
>> one with a single violation entry at the head).
>

Hi Mimi,

> What a pleasure reviewing your patch set.  Nicely organized.  Well written patch
> descriptions.

thank you :)

> Currently with the SHA1 hash algorithm, whether it is being extended into the
> TPM or not, the measurement list is complete.  Relying on the ima_hash in the
> current kernel and the subsequent kexec'ed kernel should be fine, assuming if
> they're different hash algorithms both TPM banks are enabled.  Otherwise, the
> measurement lists will be incomplete.

Yes. However with your comment I'm now realizing there's an issue if the
set of supported hash algorithms differs between the previous and the
next, kexeced kernel -- something I admittedly hadn't thought of before.

The current behavior as implemented in this RFC is that an unsupported
PCR bank would get invalidated *once* upon first use, i.e. extended once
with e.g. all 0xFEs. (Note that the actual patch implements invalidation
with all 0xFFs, for the choice of the exact invalidation value see
below). The idea is that
a.) tools could easily recognize this by comparing the PCR bank value
    against constant HASH(00 .. 00 | fe ... fe)
b.) and they would fail to verify any non-trivial event log against such
    a PCR bank if they did not do that comparison ahead.

In order to implement this invalidate-once logic, there's that
ima_extended_pcrs_mask you asked about in reply to [3/7], the
preparatory patch for [4/7] ("ima: track the set of PCRs ever
extended"). As the set of PCRs ever to be found in any policy rule
cannot be predicted, their unsupported banks cannot get invalidated once
at __init. Hence this inalidate-at-first-extend logic, which needs that
tracking of PCRs ever extended as maintained in ima_extended_pcrs_mask.

Upon kexec, the current patchset attempts to restore the
ima_extended_pcrs_mask from the previous kernel by walking through the
measurement list, setting a bit for each PCR found in any event.

Now consider the following:
- some hash algorithm is supported by the initially booted kernel,
- but not in the subsequently kexeced one.

The initially booted kernel would not invalidate the given hash
algorithm's bank for any PCR, and the kexeced one would neither, because
it would restore the ima_extended_pcrs_mask from the initially booted
one. However, the kexeced kernel would also not extend any further
events into the now unsupported PCR banks then. That means that these
PCR banks would happily verify a measurement list truncated to the point
before the kexec, which is of course bad.


I can see two ways around this:
a.) Give up on the invalidate-once scheme, unconditionally invalidate
    unsupported banks (with 0xfe .. fe) for every new measurement list
    entry.

b.) Make the kexeced kernel to read back PCR banks it doesn't support
    from the TPM at __init and see if they had been invalidated by the
    previous kernel. Set the bit in ima_extended_pcrs_mask *only* if so.
    That is, invalidate unsupported and not yet invalidated PCR banks
    upon first use.

    Also, make it read PCR banks it does support and refrain from
    further extending any found to have been invalidated before (for all
    PCRs mentioned in the measurement list). That is, leave previously
    invalidated PCR banks alone.

Going with a.) would mean that verifiers would not be able to recognize
in O(1) anymore that some bank was unsupported and had not been
maintained by the kernel. It would still be possible to figure in linear
time whether neither of the kernels in a kexec chain covered by a single
measurement list did support a given PCR bank hash.

For implementing b.), one would have to store a table of precomputed
HASH(00 .. 00 | fe .. fe) values for every recognized hash possible in
.rodata for comparison purposes, i.e. for every entry in
tpm2_hash_map[5] at least -- after all, the whole point is to deal with
hashes for which no implementation is available to the kernel, so these
values cannot get computed dynamically at runtime.

With that, if the initially booted kernel did not support some hash
algorithm, it would be recognizable by verifiers in O(1) time.

If the initially booted kernel did support a given hash, but a
subsequent kernel in the kexec chain would not, the PCR would get
invalidated by the latter. This sitatuation cannot be detected at all
(with reasonable effort) from the final PCR hash bank value alone and
verification against it would fail then. Perhaps it's noteworthy that
this is true with any possible scheme, including the currently
implemented one extending with padded SHA1 into unsupported banks.


I think that the decision about what to do now boils down to whether
there's any value in verifiers being able to tell that a PCR bank had
been unsupported and not been maintained rather than to simply fail its
verification if attempted.

If it is not important, or linear time + the additional implementation
complexity burden at the verifier side is acceptable, the much simpler
a.) would do.

Otherwise I could give implementing b.) a try and see how bad the
resulting code would get.

What do you think?


> This patch set introduces a new definition of integrity violation. Previously it
> was limited to open-writers and ToMToU integrity violations.  Now it could also
> mean no kernel hash algorithm available.  Unfortunately some attestation
> services simply ignore integrity violations.

Yeah, there's indeed an ambiguity. I think the right thing to do is to
make measurement lists unverifiable against unsupported banks and would
propose to use 0xfe ... fe for the associated invalidations instead of
the 0xff .. ff used for violation events already.

Thanks a lot!

Nicolai


>> 
>> So implement d.). As it potentially breaks existing userspace, i.e.
>> the current implementation of ima_measurement, put it behind a Kconfig
>> option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
>> behavior of extending with padded SHA-1 is retained. Otherwise the new
>> scheme to invalidate unsupported PCR banks once upon their first extension
>> from IMA is implemented instead. As ima_measurement is marked as
>> experimental and I find it unlikely that other existing tools depend on
>> the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
>> Kconfig option default to "n".
>> 
>> For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
>> - make ima_calc_field_array_hash() to fill the digests corresponding to
>>   banks with unsupported hash algorithms with 0xffs,
>> - make ima_pcr_extend() to extend these into the unsupported PCR banks only
>>   upon the PCR's first usage, skip them on subsequent updates and
>> - let ima_init_ima_crypto() help it with that by populating the new
>>   ima_unsupported_tpm_banks_mask with one bit set for each bank with
>>   an unavailable hash algorithm at init.
>> 
>> [1] https://github.com/linux-integrity/ima-evm-utils
>> 
>> Signed-off-by: Nicolai Stange <nstange@suse.de>
>> ---

-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [RFC PATCH v1 0/7] ima: get rid of hard dependency on SHA-1

[Roberto Sassu]

On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
> Hi all,
> 
> if no SHA-1 implementation was available to the kernel, IMA init would
> currently fail, rendering the whole subsystem unusable.
> 
> This patch series is an attempt to make SHA-1 availability non-mandatory
> for IMA. The main motivation is that NIST announced to sunset SHA-1 by
> 2030 ([1]), whereby any attempt to instantiate it when booted in FIPS mode
> would have to be made to fail with -ENOENT. As this does potentially have
> an impact on lifetimes for FIPS certifications issued today, distros might
> be interested in disabling SHA-1 downstream soon already.
> 
> Anyway, making IMA to work without a SHA-1 implementation available is not
> so straightforward, mainly due to that established scheme to substitute
> padded SHA-1 template hashes for PCR banks with unmapped/unavailable algos.
> There is some userspace around expecting that existing behavior, e.g. the
> ima_measurement command from ([2]), and breaking that in certain scenarios
> is inevitable.
> 
> I tried to make it the least painful possible, and I think I arrived at
> a not completely unreasonable solution in the end, but wouldn't be too
> surprised if you had a different stance on that. So I would be curious
> about your feedback on whether this is a route worth pursuing any further.
> FWIW, the most controversial parts are probably
>  - [1/7] ima: don't expose runtime_measurements for unsupported hashes
>  - [6/7] ima: invalidate unsupported PCR banks once at first use
> 
> Note that I haven't tested this series thoroughly yet -- for the time being
> I only ran a couple of brief smoke tests in a VM w/o a TPM  (w/ and w/o
> SHA-1 disabled of course).

+ Jarkko

Hi Nicolai

thanks a lot for the patches. Still didn't go through them, but if I
understood correctly you assume that the SHA1 PCR bank would be still
seen by IMA.

In light of deprecation of SHA1, is this assumption correct?

I would expect that TPM manufacturers or even the TPM driver would
change to fullfill that.

I guess the first stage would be making sure that the SHA1 PCR bank is
unusable at the TPM driver level. A first thought would be to extend
the SHA1 PCR bank with a random value at boot (or earlier), so that the
remote attestation would never work on that PCR bank. At that point, I
would probably go further and not expose the SHA1 PCR bank at all, so
you would have less problems on IMA side.

The second stage would probably be that the TPM firmware would be
updated, not allowing the SHA1 PCR bank to be allocated.

Other than that, sure, also actions need to be done to remove SHA1
support in IMA (will look at your patches).

Roberto

> Many thanks!
> 
> Nicolai
> 
> [1] https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm
> [2] https://github.com/linux-integrity/ima-evm-utils.git
> 
> Nicolai Stange (7):
>   ima: don't expose runtime_measurements for unsupported hashes
>   ima: always create runtime_measurements sysfs file for ima_hash
>   ima: move INVALID_PCR() to ima.h
>   ima: track the set of PCRs ever extended
>   tpm: enable bank selection for PCR extend
>   ima: invalidate unsupported PCR banks once at first use
>   ima: make SHA1 non-mandatory
> 
>  drivers/char/tpm/tpm-interface.c      | 29 +++++++++-
>  drivers/char/tpm/tpm.h                |  3 +-
>  drivers/char/tpm/tpm2-cmd.c           | 29 +++++++++-
>  include/linux/tpm.h                   |  3 +
>  security/integrity/ima/Kconfig        | 14 +++++
>  security/integrity/ima/ima.h          |  9 +++
>  security/integrity/ima/ima_crypto.c   | 83 ++++++++++++++++-----------
>  security/integrity/ima/ima_fs.c       | 41 +++++++------
>  security/integrity/ima/ima_policy.c   |  5 +-
>  security/integrity/ima/ima_queue.c    | 26 ++++++++-
>  security/integrity/ima/ima_template.c |  7 +++
>  11 files changed, 190 insertions(+), 59 deletions(-)
> 

Re: [RFC PATCH v1 0/7] ima: get rid of hard dependency on SHA-1

[Nicolai Stange]

Roberto Sassu <roberto.sassu@huaweicloud.com> writes:

> On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
>> Hi all,
>> 
>> if no SHA-1 implementation was available to the kernel, IMA init would
>> currently fail, rendering the whole subsystem unusable.
>> 
>> This patch series is an attempt to make SHA-1 availability non-mandatory
>> for IMA. The main motivation is that NIST announced to sunset SHA-1 by
>> 2030 ([1]), whereby any attempt to instantiate it when booted in FIPS mode
>> would have to be made to fail with -ENOENT. As this does potentially have
>> an impact on lifetimes for FIPS certifications issued today, distros might
>> be interested in disabling SHA-1 downstream soon already.
>> 
>> Anyway, making IMA to work without a SHA-1 implementation available is not
>> so straightforward, mainly due to that established scheme to substitute
>> padded SHA-1 template hashes for PCR banks with unmapped/unavailable algos.
>> There is some userspace around expecting that existing behavior, e.g. the
>> ima_measurement command from ([2]), and breaking that in certain scenarios
>> is inevitable.
>> 
>> I tried to make it the least painful possible, and I think I arrived at
>> a not completely unreasonable solution in the end, but wouldn't be too
>> surprised if you had a different stance on that. So I would be curious
>> about your feedback on whether this is a route worth pursuing any further.
>> FWIW, the most controversial parts are probably
>>  - [1/7] ima: don't expose runtime_measurements for unsupported hashes
>>  - [6/7] ima: invalidate unsupported PCR banks once at first use
>> 
>> Note that I haven't tested this series thoroughly yet -- for the time being
>> I only ran a couple of brief smoke tests in a VM w/o a TPM  (w/ and w/o
>> SHA-1 disabled of course).
>

Hi Roberto,

> thanks a lot for the patches. Still didn't go through them, but if I
> understood correctly you assume that the SHA1 PCR bank would be still
> seen by IMA.
>
> In light of deprecation of SHA1, is this assumption correct?

yes, the assumption made here is that a SHA-1 TPM bank might exist and
is visible to IMA, but that the kernel would not have a working SHA-1
implementation available.

>
> I would expect that TPM manufacturers or even the TPM driver would
> change to fullfill that.
>
> I guess the first stage would be making sure that the SHA1 PCR bank is
> unusable at the TPM driver level. A first thought would be to extend
> the SHA1 PCR bank with a random value at boot (or earlier), so that the
> remote attestation would never work on that PCR bank. At that point, I
> would probably go further and not expose the SHA1 PCR bank at all, so
> you would have less problems on IMA side.

I would like to note in this context that from my POV there's nothing
really special about SHA-1 when compared to any other potential TPM bank
hash algos the kernel doesn't have an implementation for. That is, if a
TPM implemented say SHA3-256 and the kernel did not have an
implementation of that built-in, it would be a very similar situation as
far as IMA is concerned, i.e. it would have to get handled somehow.

Thanks!

Nicolai

>
> The second stage would probably be that the TPM firmware would be
> updated, not allowing the SHA1 PCR bank to be allocated.
>
> Other than that, sure, also actions need to be done to remove SHA1
> support in IMA (will look at your patches).
>
>> 
>> [1] https://www.nist.gov/news-events/news/2022/12/nist-retires-sha-1-cryptographic-algorithm
>> [2] https://github.com/linux-integrity/ima-evm-utils.git
>> 

-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Mimi Zohar]

On Tue, 2025-03-18 at 11:26 +0100, Nicolai Stange wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
> 
> > On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
> > > Normally IMA would extend a template hash of each bank's associated
> > > algorithm into a PCR. However, if a bank's hash algorithm is unavailable
> > > to the kernel at IMA init time, it would fallback to extending padded
> > > SHA1 hashes instead.
> > > 
> > > That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
> > > SHA1 template hashes into a PCR's SHA-256 bank.
> > > 
> > > The ima_measurement command (marked as experimental) from ima-evm-utils
> > > would accordingly try both variants when attempting to verify a measurement
> > > list against PCRs. keylime OTOH doesn't seem to -- it expects the template
> > > hash type to match the PCR bank algorithm. I would argue that for the
> > > latter case, the fallback scheme could potentially cause hard to debug
> > > verification failures.
> > > 
> > > There's another problem with the fallback scheme: right now, SHA-1
> > > availability is a hard requirement for IMA, and it would be good for a
> > > number of reasons to get rid of that. However, if SHA-1 is not available to
> > > the kernel, it can hardly provide padded SHA-1 template hashes for PCR
> > > banks with unsupported algos.
> > > 
> > > There are several more or less reasonable alternatives possible, among
> > > them are:
> > > a.) Instead of padded SHA-1, use padded/truncated ima_hash template
> > >     hashes.
> > > b.) Record every event as a violation, i.e. extend unsupported banks
> > >     with 0xffs.
> > > c.) Don't extend unsupported banks at all.
> > > d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
> > >     use.
> > > 
> > > a.) would make verification from tools like ima_measurement nearly
> > >     impossible, as it would have to guess or somehow determine ima_hash.
> > > b.) would still put an significant and unnecessary burden on tools like
> > >     ima_measurement, because it would then have to exercise three
> > >     possible variants on the measurement list:
> > >     - the template hash matches the bank algorithm,
> > >     - the template hash is padded SHA-1,
> > >     - the template hash is all-ones.
> > > c.) is a security risk, because the bank would validate an empty
> > >     measurement list.
> > > 
> > > AFAICS, d.) is the best option to proceed, as it allows for determining
> > > from the PCR bank value in O(1) whether the bank had been maintained by
> > > IMA or not and also, it would not validate any measurement list (except
> > > one with a single violation entry at the head).
> > 
> 
> Hi Mimi,
> 
> > What a pleasure reviewing your patch set.  Nicely organized.  Well written patch
> > descriptions.
> 
> thank you :)
> 
> > Currently with the SHA1 hash algorithm, whether it is being extended into the
> > TPM or not, the measurement list is complete.  Relying on the ima_hash in the
> > current kernel and the subsequent kexec'ed kernel should be fine, assuming if
> > they're different hash algorithms both TPM banks are enabled.  Otherwise, the
> > measurement lists will be incomplete.
> 
> Yes. However with your comment I'm now realizing there's an issue if the
> set of supported hash algorithms differs between the previous and the
> next, kexeced kernel -- something I admittedly hadn't thought of before.
> 
> The current behavior as implemented in this RFC is that an unsupported
> PCR bank would get invalidated *once* upon first use, i.e. extended once
> with e.g. all 0xFEs. (Note that the actual patch implements invalidation
> with all 0xFFs, for the choice of the exact invalidation value see
> below). The idea is that
> a.) tools could easily recognize this by comparing the PCR bank value
>     against constant HASH(00 .. 00 | fe ... fe)
> b.) and they would fail to verify any non-trivial event log against such
>     a PCR bank if they did not do that comparison ahead.
> 
> In order to implement this invalidate-once logic, there's that
> ima_extended_pcrs_mask you asked about in reply to [3/7], the
> preparatory patch for [4/7] ("ima: track the set of PCRs ever
> extended"). As the set of PCRs ever to be found in any policy rule
> cannot be predicted, their unsupported banks cannot get invalidated once
> at __init. Hence this inalidate-at-first-extend logic, which needs that
> tracking of PCRs ever extended as maintained in ima_extended_pcrs_mask.
> 
> Upon kexec, the current patchset attempts to restore the
> ima_extended_pcrs_mask from the previous kernel by walking through the
> measurement list, setting a bit for each PCR found in any event.
> 
> Now consider the following:
> - some hash algorithm is supported by the initially booted kernel,
> - but not in the subsequently kexeced one.
> 
> The initially booted kernel would not invalidate the given hash
> algorithm's bank for any PCR, and the kexeced one would neither, because
> it would restore the ima_extended_pcrs_mask from the initially booted
> one. However, the kexeced kernel would also not extend any further
> events into the now unsupported PCR banks then. That means that these
> PCR banks would happily verify a measurement list truncated to the point
> before the kexec, which is of course bad.
> 
> 
> I can see two ways around this:
> a.) Give up on the invalidate-once scheme, unconditionally invalidate
>     unsupported banks (with 0xfe .. fe) for every new measurement list
>     entry.
> 
> b.) Make the kexeced kernel to read back PCR banks it doesn't support
>     from the TPM at __init and see if they had been invalidated by the
>     previous kernel. Set the bit in ima_extended_pcrs_mask *only* if so.
>     That is, invalidate unsupported and not yet invalidated PCR banks
>     upon first use.
> 
>     Also, make it read PCR banks it does support and refrain from
>     further extending any found to have been invalidated before (for all
>     PCRs mentioned in the measurement list). That is, leave previously
>     invalidated PCR banks alone.
> 
> Going with a.) would mean that verifiers would not be able to recognize
> in O(1) anymore that some bank was unsupported and had not been
> maintained by the kernel. It would still be possible to figure in linear
> time whether neither of the kernels in a kexec chain covered by a single
> measurement list did support a given PCR bank hash.
> 
> For implementing b.), one would have to store a table of precomputed
> HASH(00 .. 00 | fe .. fe) values for every recognized hash possible in
> .rodata for comparison purposes, i.e. for every entry in
> tpm2_hash_map[5] at least -- after all, the whole point is to deal with
> hashes for which no implementation is available to the kernel, so these
> values cannot get computed dynamically at runtime.
> 
> With that, if the initially booted kernel did not support some hash
> algorithm, it would be recognizable by verifiers in O(1) time.
> 
> If the initially booted kernel did support a given hash, but a
> subsequent kernel in the kexec chain would not, the PCR would get
> invalidated by the latter. This sitatuation cannot be detected at all
> (with reasonable effort) from the final PCR hash bank value alone and
> verification against it would fail then. Perhaps it's noteworthy that
> this is true with any possible scheme, including the currently
> implemented one extending with padded SHA1 into unsupported banks.
> 
> 
> I think that the decision about what to do now boils down to whether
> there's any value in verifiers being able to tell that a PCR bank had
> been unsupported and not been maintained rather than to simply fail its
> verification if attempted.
> 
> If it is not important, or linear time + the additional implementation
> complexity burden at the verifier side is acceptable, the much simpler
> a.) would do.
> 
> Otherwise I could give implementing b.) a try and see how bad the
> resulting code would get.
> 
> What do you think?

Let me try to summarize 'b'.  The initial unsupported hash algorithms would
continue to be unsupported in subsequent kexec's.  However this does not address
the case where the initial kernel image supported a hash algorithm, but the
subsequent kexec'ed image does not.  The TPM bank has already been extended with
other values.  In this case, like the original violation the attestation service
would not verify.  If I'm understanding it correctly, 'b' is thus a partial
solution.

My concern with 'b' is the ability to read the multiple TPM bank PCRs so early
during kernel initialization.  Will it succeed?  If it does succeed, will it
introduce initialization delays?

FYI, because the IMA Kconfig selects SHA1, we're guaranteed that SHA1 exists in
the kernel and the subsequent kexec'ed kernel.  For this reason we're guaranteed
that the measurement list is complete.  The simplest solution, not necessarily
the best, would be to punt the problem for the time being by replacing the
"select" with a different hash algorithm.

> 
> 
> > This patch set introduces a new definition of integrity violation. Previously it
> > was limited to open-writers and ToMToU integrity violations.  Now it could also
> > mean no kernel hash algorithm available.  Unfortunately some attestation
> > services simply ignore integrity violations.
> 
> Yeah, there's indeed an ambiguity. I think the right thing to do is to
> make measurement lists unverifiable against unsupported banks and would
> propose to use 0xfe ... fe for the associated invalidations instead of
> the 0xff .. ff used for violation events already.

I just realized that unlike the existing open-writers/ToMToU violations, by
definition the new unsupported bank violation would not be included in the
measurement list, but just extended into the TPM.

Mimi

> 
> > > 
> > > So implement d.). As it potentially breaks existing userspace, i.e.
> > > the current implementation of ima_measurement, put it behind a Kconfig
> > > option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
> > > behavior of extending with padded SHA-1 is retained. Otherwise the new
> > > scheme to invalidate unsupported PCR banks once upon their first extension
> > > from IMA is implemented instead. As ima_measurement is marked as
> > > experimental and I find it unlikely that other existing tools depend on
> > > the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
> > > Kconfig option default to "n".
> > > 
> > > For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
> > > - make ima_calc_field_array_hash() to fill the digests corresponding to
> > >   banks with unsupported hash algorithms with 0xffs,
> > > - make ima_pcr_extend() to extend these into the unsupported PCR banks only
> > >   upon the PCR's first usage, skip them on subsequent updates and
> > > - let ima_init_ima_crypto() help it with that by populating the new
> > >   ima_unsupported_tpm_banks_mask with one bit set for each bank with
> > >   an unavailable hash algorithm at init.
> > > 
> > > [1] https://github.com/linux-integrity/ima-evm-utils
> > > 
> > > Signed-off-by: Nicolai Stange <nstange@suse.de>
> > > ---
> 

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Nicolai Stange]

Mimi Zohar <zohar@linux.ibm.com> writes:

> On Tue, 2025-03-18 at 11:26 +0100, Nicolai Stange wrote:
>> Mimi Zohar <zohar@linux.ibm.com> writes:
>> 
>> > On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
>> > > Normally IMA would extend a template hash of each bank's associated
>> > > algorithm into a PCR. However, if a bank's hash algorithm is unavailable
>> > > to the kernel at IMA init time, it would fallback to extending padded
>> > > SHA1 hashes instead.
>> > > 
>> > > That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
>> > > SHA1 template hashes into a PCR's SHA-256 bank.
>> > > 
>> > > The ima_measurement command (marked as experimental) from ima-evm-utils
>> > > would accordingly try both variants when attempting to verify a measurement
>> > > list against PCRs. keylime OTOH doesn't seem to -- it expects the template
>> > > hash type to match the PCR bank algorithm. I would argue that for the
>> > > latter case, the fallback scheme could potentially cause hard to debug
>> > > verification failures.
>> > > 
>> > > There's another problem with the fallback scheme: right now, SHA-1
>> > > availability is a hard requirement for IMA, and it would be good for a
>> > > number of reasons to get rid of that. However, if SHA-1 is not available to
>> > > the kernel, it can hardly provide padded SHA-1 template hashes for PCR
>> > > banks with unsupported algos.
>> > > 
>> > > There are several more or less reasonable alternatives possible, among
>> > > them are:
>> > > a.) Instead of padded SHA-1, use padded/truncated ima_hash template
>> > >     hashes.
>> > > b.) Record every event as a violation, i.e. extend unsupported banks
>> > >     with 0xffs.
>> > > c.) Don't extend unsupported banks at all.
>> > > d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
>> > >     use.
>> > > 
>> > > a.) would make verification from tools like ima_measurement nearly
>> > >     impossible, as it would have to guess or somehow determine ima_hash.
>> > > b.) would still put an significant and unnecessary burden on tools like
>> > >     ima_measurement, because it would then have to exercise three
>> > >     possible variants on the measurement list:
>> > >     - the template hash matches the bank algorithm,
>> > >     - the template hash is padded SHA-1,
>> > >     - the template hash is all-ones.
>> > > c.) is a security risk, because the bank would validate an empty
>> > >     measurement list.
>> > > 
>> > > AFAICS, d.) is the best option to proceed, as it allows for determining
>> > > from the PCR bank value in O(1) whether the bank had been maintained by
>> > > IMA or not and also, it would not validate any measurement list (except
>> > > one with a single violation entry at the head).
>> > 
>> 
>> Hi Mimi,
>> 
>> > What a pleasure reviewing your patch set.  Nicely organized.  Well written patch
>> > descriptions.
>> 
>> thank you :)
>> 
>> > Currently with the SHA1 hash algorithm, whether it is being extended into the
>> > TPM or not, the measurement list is complete.  Relying on the ima_hash in the
>> > current kernel and the subsequent kexec'ed kernel should be fine, assuming if
>> > they're different hash algorithms both TPM banks are enabled.  Otherwise, the
>> > measurement lists will be incomplete.
>> 
>> Yes. However with your comment I'm now realizing there's an issue if the
>> set of supported hash algorithms differs between the previous and the
>> next, kexeced kernel -- something I admittedly hadn't thought of before.
>> 
>> The current behavior as implemented in this RFC is that an unsupported
>> PCR bank would get invalidated *once* upon first use, i.e. extended once
>> with e.g. all 0xFEs. (Note that the actual patch implements invalidation
>> with all 0xFFs, for the choice of the exact invalidation value see
>> below). The idea is that
>> a.) tools could easily recognize this by comparing the PCR bank value
>>     against constant HASH(00 .. 00 | fe ... fe)
>> b.) and they would fail to verify any non-trivial event log against such
>>     a PCR bank if they did not do that comparison ahead.
>> 
>> In order to implement this invalidate-once logic, there's that
>> ima_extended_pcrs_mask you asked about in reply to [3/7], the
>> preparatory patch for [4/7] ("ima: track the set of PCRs ever
>> extended"). As the set of PCRs ever to be found in any policy rule
>> cannot be predicted, their unsupported banks cannot get invalidated once
>> at __init. Hence this inalidate-at-first-extend logic, which needs that
>> tracking of PCRs ever extended as maintained in ima_extended_pcrs_mask.
>> 
>> Upon kexec, the current patchset attempts to restore the
>> ima_extended_pcrs_mask from the previous kernel by walking through the
>> measurement list, setting a bit for each PCR found in any event.
>> 
>> Now consider the following:
>> - some hash algorithm is supported by the initially booted kernel,
>> - but not in the subsequently kexeced one.
>> 
>> The initially booted kernel would not invalidate the given hash
>> algorithm's bank for any PCR, and the kexeced one would neither, because
>> it would restore the ima_extended_pcrs_mask from the initially booted
>> one. However, the kexeced kernel would also not extend any further
>> events into the now unsupported PCR banks then. That means that these
>> PCR banks would happily verify a measurement list truncated to the point
>> before the kexec, which is of course bad.
>> 
>> 
>> I can see two ways around this:
>> a.) Give up on the invalidate-once scheme, unconditionally invalidate
>>     unsupported banks (with 0xfe .. fe) for every new measurement list
>>     entry.
>> 
>> b.) Make the kexeced kernel to read back PCR banks it doesn't support
>>     from the TPM at __init and see if they had been invalidated by the
>>     previous kernel. Set the bit in ima_extended_pcrs_mask *only* if so.
>>     That is, invalidate unsupported and not yet invalidated PCR banks
>>     upon first use.
>> 
>>     Also, make it read PCR banks it does support and refrain from
>>     further extending any found to have been invalidated before (for all
>>     PCRs mentioned in the measurement list). That is, leave previously
>>     invalidated PCR banks alone.
>> 
>> Going with a.) would mean that verifiers would not be able to recognize
>> in O(1) anymore that some bank was unsupported and had not been
>> maintained by the kernel. It would still be possible to figure in linear
>> time whether neither of the kernels in a kexec chain covered by a single
>> measurement list did support a given PCR bank hash.
>> 
>> For implementing b.), one would have to store a table of precomputed
>> HASH(00 .. 00 | fe .. fe) values for every recognized hash possible in
>> .rodata for comparison purposes, i.e. for every entry in
>> tpm2_hash_map[5] at least -- after all, the whole point is to deal with
>> hashes for which no implementation is available to the kernel, so these
>> values cannot get computed dynamically at runtime.
>> 
>> With that, if the initially booted kernel did not support some hash
>> algorithm, it would be recognizable by verifiers in O(1) time.
>> 
>> If the initially booted kernel did support a given hash, but a
>> subsequent kernel in the kexec chain would not, the PCR would get
>> invalidated by the latter. This sitatuation cannot be detected at all
>> (with reasonable effort) from the final PCR hash bank value alone and
>> verification against it would fail then. Perhaps it's noteworthy that
>> this is true with any possible scheme, including the currently
>> implemented one extending with padded SHA1 into unsupported banks.
>> 
>> 
>> I think that the decision about what to do now boils down to whether
>> there's any value in verifiers being able to tell that a PCR bank had
>> been unsupported and not been maintained rather than to simply fail its
>> verification if attempted.
>> 
>> If it is not important, or linear time + the additional implementation
>> complexity burden at the verifier side is acceptable, the much simpler
>> a.) would do.
>> 
>> Otherwise I could give implementing b.) a try and see how bad the
>> resulting code would get.
>> 
>> What do you think?
>
> Let me try to summarize 'b'.  The initial unsupported hash algorithms would
> continue to be unsupported in subsequent kexec's.  However this does not address
> the case where the initial kernel image supported a hash algorithm, but the
> subsequent kexec'ed image does not.  The TPM bank has already been extended with
> other values.  In this case, like the original violation the attestation service
> would not verify.  If I'm understanding it correctly, 'b' is thus a partial
> solution.

Yes, that all matches exactly what I was trying to say. FWIW, I might be
way too naive, but I would expect two categories of existing verifier
behaviors:
- "Real" ones like keylime or so, which are being asked to verify
  against a single specific bank and the result is either yes or no with
  no inbetween.  In particular, these wouldn't fallback to checking
  whether something else like padded SHA1s would perhaps verify.
- The ones more in the development/debugging/testsuite realm, which
  would attempt to verify against all banks and fail (the test?) if any
  does not verify. These would try harder to avoid false negatives by
  testing for the alternatives like padded SHA1s as well. I suppose
  ima-evm-utils' ima_measurement would qualify as such one.

For the first class, simply invalidating the unsupported PCR banks
somehow, i.e. option a.), is good enough. For the second kind however,
the question is whether something like b.) would be helpful and the
additional complexity on the kernel side warranted. But agreed, it's a
partial and best-effort solution. If one kexecs into a kernel with a
proper subset of supported TPM bank hashes, it wouldnt't work.


> My concern with 'b' is the ability to read the multiple TPM bank PCRs so early
> during kernel initialization.  Will it succeed?  If it does succeed, will it
> introduce initialization delays?

As the boot aggregate gets extended already at that point in time
(IIRC), I'd expect that reading the PCRs would probably succeed as
well. For the delays imposed on the kexec restore path -- I can't tell
unfortunately. But I would only do it on kexec restore if the
measurement list is non-empty anyway, so systems not having IMA enabled
or ones which wouldn't kexec are not affected.


> FYI, because the IMA Kconfig selects SHA1, we're guaranteed that SHA1 exists in
> the kernel and the subsequent kexec'ed kernel.  For this reason we're guaranteed
> that the measurement list is complete.  The simplest solution, not necessarily
> the best, would be to punt the problem for the time being by replacing the
> "select" with a different hash algorithm.

Yes, that would work as well. IIUC, it would mean that we would
e.g. extend truncated SHA-256 template hashes into a SHA-1 bank, right?
However, since no existing tool like 'ima_measurement' is expecting
that, and would fail a verification then, I'm currently struggling to
see the advantage over just doing a.) and invalidating the PCR banks
with a fixed value right away?


>> > This patch set introduces a new definition of integrity violation. Previously it
>> > was limited to open-writers and ToMToU integrity violations.  Now it could also
>> > mean no kernel hash algorithm available.  Unfortunately some attestation
>> > services simply ignore integrity violations.
>> 
>> Yeah, there's indeed an ambiguity. I think the right thing to do is to
>> make measurement lists unverifiable against unsupported banks and would
>> propose to use 0xfe ... fe for the associated invalidations instead of
>> the 0xff .. ff used for violation events already.
>
> I just realized that unlike the existing open-writers/ToMToU violations, by
> definition the new unsupported bank violation would not be included in the
> measurement list, but just extended into the TPM.

That's true, but when invalidating unsupported banks with a single ff
... ff, one could successfully verify a measurement list having only a
single violation entry against an invalidated bank AFAICS. I think it
would be more robust to use a different constant for the invalidation.

Thanks!

Nicolai


>> > > So implement d.). As it potentially breaks existing userspace, i.e.
>> > > the current implementation of ima_measurement, put it behind a Kconfig
>> > > option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
>> > > behavior of extending with padded SHA-1 is retained. Otherwise the new
>> > > scheme to invalidate unsupported PCR banks once upon their first extension
>> > > from IMA is implemented instead. As ima_measurement is marked as
>> > > experimental and I find it unlikely that other existing tools depend on
>> > > the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
>> > > Kconfig option default to "n".
>> > > 
>> > > For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
>> > > - make ima_calc_field_array_hash() to fill the digests corresponding to
>> > >   banks with unsupported hash algorithms with 0xffs,
>> > > - make ima_pcr_extend() to extend these into the unsupported PCR banks only
>> > >   upon the PCR's first usage, skip them on subsequent updates and
>> > > - let ima_init_ima_crypto() help it with that by populating the new
>> > >   ima_unsupported_tpm_banks_mask with one bit set for each bank with
>> > >   an unavailable hash algorithm at init.
>> > > 
>> > > [1] https://github.com/linux-integrity/ima-evm-utils
>> > > 
>> > > Signed-off-by: Nicolai Stange <nstange@suse.de>
>> > > ---
>> 
>

-- 
SUSE Software Solutions Germany GmbH, Frankenstraße 146, 90461 Nürnberg, Germany
GF: Ivo Totev, Andrew McDonald, Werner Knoblich
(HRB 36809, AG Nürnberg)

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Mimi Zohar]

On Tue, 2025-03-18 at 16:55 +0100, Nicolai Stange wrote:
> Mimi Zohar <zohar@linux.ibm.com> writes:
> 
> > On Tue, 2025-03-18 at 11:26 +0100, Nicolai Stange wrote:
> > > Mimi Zohar <zohar@linux.ibm.com> writes:
> > > 
> > > > On Thu, 2025-03-13 at 18:33 +0100, Nicolai Stange wrote:
> > > > > Normally IMA would extend a template hash of each bank's associated
> > > > > algorithm into a PCR. However, if a bank's hash algorithm is unavailable
> > > > > to the kernel at IMA init time, it would fallback to extending padded
> > > > > SHA1 hashes instead.
> > > > > 
> > > > > That is, if e.g. SHA-256 was missing at IMA init, it would extend padded
> > > > > SHA1 template hashes into a PCR's SHA-256 bank.
> > > > > 
> > > > > The ima_measurement command (marked as experimental) from ima-evm-utils
> > > > > would accordingly try both variants when attempting to verify a measurement
> > > > > list against PCRs. keylime OTOH doesn't seem to -- it expects the template
> > > > > hash type to match the PCR bank algorithm. I would argue that for the
> > > > > latter case, the fallback scheme could potentially cause hard to debug
> > > > > verification failures.
> > > > > 
> > > > > There's another problem with the fallback scheme: right now, SHA-1
> > > > > availability is a hard requirement for IMA, and it would be good for a
> > > > > number of reasons to get rid of that. However, if SHA-1 is not available to
> > > > > the kernel, it can hardly provide padded SHA-1 template hashes for PCR
> > > > > banks with unsupported algos.
> > > > > 
> > > > > There are several more or less reasonable alternatives possible, among
> > > > > them are:
> > > > > a.) Instead of padded SHA-1, use padded/truncated ima_hash template
> > > > >     hashes.
> > > > > b.) Record every event as a violation, i.e. extend unsupported banks
> > > > >     with 0xffs.
> > > > > c.) Don't extend unsupported banks at all.
> > > > > d.) Invalidate unsupported banks only once (e.g. with 0xffs) at first
> > > > >     use.
> > > > > 
> > > > > a.) would make verification from tools like ima_measurement nearly
> > > > >     impossible, as it would have to guess or somehow determine ima_hash.
> > > > > b.) would still put an significant and unnecessary burden on tools like
> > > > >     ima_measurement, because it would then have to exercise three
> > > > >     possible variants on the measurement list:
> > > > >     - the template hash matches the bank algorithm,
> > > > >     - the template hash is padded SHA-1,
> > > > >     - the template hash is all-ones.
> > > > > c.) is a security risk, because the bank would validate an empty
> > > > >     measurement list.
> > > > > 
> > > > > AFAICS, d.) is the best option to proceed, as it allows for determining
> > > > > from the PCR bank value in O(1) whether the bank had been maintained by
> > > > > IMA or not and also, it would not validate any measurement list (except
> > > > > one with a single violation entry at the head).
> > > > 
> > > 
> > > Hi Mimi,
> > > 
> > > > What a pleasure reviewing your patch set.  Nicely organized.  Well written patch
> > > > descriptions.
> > > 
> > > thank you :)
> > > 
> > > > Currently with the SHA1 hash algorithm, whether it is being extended into the
> > > > TPM or not, the measurement list is complete.  Relying on the ima_hash in the
> > > > current kernel and the subsequent kexec'ed kernel should be fine, assuming if
> > > > they're different hash algorithms both TPM banks are enabled.  Otherwise, the
> > > > measurement lists will be incomplete.
> > > 
> > > Yes. However with your comment I'm now realizing there's an issue if the
> > > set of supported hash algorithms differs between the previous and the
> > > next, kexeced kernel -- something I admittedly hadn't thought of before.
> > > 
> > > The current behavior as implemented in this RFC is that an unsupported
> > > PCR bank would get invalidated *once* upon first use, i.e. extended once
> > > with e.g. all 0xFEs. (Note that the actual patch implements invalidation
> > > with all 0xFFs, for the choice of the exact invalidation value see
> > > below). The idea is that
> > > a.) tools could easily recognize this by comparing the PCR bank value
> > >     against constant HASH(00 .. 00 | fe ... fe)
> > > b.) and they would fail to verify any non-trivial event log against such
> > >     a PCR bank if they did not do that comparison ahead.
> > > 
> > > In order to implement this invalidate-once logic, there's that
> > > ima_extended_pcrs_mask you asked about in reply to [3/7], the
> > > preparatory patch for [4/7] ("ima: track the set of PCRs ever
> > > extended"). As the set of PCRs ever to be found in any policy rule
> > > cannot be predicted, their unsupported banks cannot get invalidated once
> > > at __init. Hence this inalidate-at-first-extend logic, which needs that
> > > tracking of PCRs ever extended as maintained in ima_extended_pcrs_mask.
> > > 
> > > Upon kexec, the current patchset attempts to restore the
> > > ima_extended_pcrs_mask from the previous kernel by walking through the
> > > measurement list, setting a bit for each PCR found in any event.
> > > 
> > > Now consider the following:
> > > - some hash algorithm is supported by the initially booted kernel,
> > > - but not in the subsequently kexeced one.
> > > 
> > > The initially booted kernel would not invalidate the given hash
> > > algorithm's bank for any PCR, and the kexeced one would neither, because
> > > it would restore the ima_extended_pcrs_mask from the initially booted
> > > one. However, the kexeced kernel would also not extend any further
> > > events into the now unsupported PCR banks then. That means that these
> > > PCR banks would happily verify a measurement list truncated to the point
> > > before the kexec, which is of course bad.
> > > 
> > > 
> > > I can see two ways around this:
> > > a.) Give up on the invalidate-once scheme, unconditionally invalidate
> > >     unsupported banks (with 0xfe .. fe) for every new measurement list
> > >     entry.
> > > 
> > > b.) Make the kexeced kernel to read back PCR banks it doesn't support
> > >     from the TPM at __init and see if they had been invalidated by the
> > >     previous kernel. Set the bit in ima_extended_pcrs_mask *only* if so.
> > >     That is, invalidate unsupported and not yet invalidated PCR banks
> > >     upon first use.
> > > 
> > >     Also, make it read PCR banks it does support and refrain from
> > >     further extending any found to have been invalidated before (for all
> > >     PCRs mentioned in the measurement list). That is, leave previously
> > >     invalidated PCR banks alone.
> > > 
> > > Going with a.) would mean that verifiers would not be able to recognize
> > > in O(1) anymore that some bank was unsupported and had not been
> > > maintained by the kernel. It would still be possible to figure in linear
> > > time whether neither of the kernels in a kexec chain covered by a single
> > > measurement list did support a given PCR bank hash.
> > > 
> > > For implementing b.), one would have to store a table of precomputed
> > > HASH(00 .. 00 | fe .. fe) values for every recognized hash possible in
> > > .rodata for comparison purposes, i.e. for every entry in
> > > tpm2_hash_map[5] at least -- after all, the whole point is to deal with
> > > hashes for which no implementation is available to the kernel, so these
> > > values cannot get computed dynamically at runtime.
> > > 
> > > With that, if the initially booted kernel did not support some hash
> > > algorithm, it would be recognizable by verifiers in O(1) time.
> > > 
> > > If the initially booted kernel did support a given hash, but a
> > > subsequent kernel in the kexec chain would not, the PCR would get
> > > invalidated by the latter. This sitatuation cannot be detected at all
> > > (with reasonable effort) from the final PCR hash bank value alone and
> > > verification against it would fail then. Perhaps it's noteworthy that
> > > this is true with any possible scheme, including the currently
> > > implemented one extending with padded SHA1 into unsupported banks.
> > > 
> > > 
> > > I think that the decision about what to do now boils down to whether
> > > there's any value in verifiers being able to tell that a PCR bank had
> > > been unsupported and not been maintained rather than to simply fail its
> > > verification if attempted.
> > > 
> > > If it is not important, or linear time + the additional implementation
> > > complexity burden at the verifier side is acceptable, the much simpler
> > > a.) would do.
> > > 
> > > Otherwise I could give implementing b.) a try and see how bad the
> > > resulting code would get.
> > > 
> > > What do you think?
> > 
> > Let me try to summarize 'b'.  The initial unsupported hash algorithms would
> > continue to be unsupported in subsequent kexec's.  However this does not address
> > the case where the initial kernel image supported a hash algorithm, but the
> > subsequent kexec'ed image does not.  The TPM bank has already been extended with
> > other values.  In this case, like the original violation the attestation service
> > would not verify.  If I'm understanding it correctly, 'b' is thus a partial
> > solution.
> 
> Yes, that all matches exactly what I was trying to say. FWIW, I might be
> way too naive, but I would expect two categories of existing verifier
> behaviors:
> - "Real" ones like keylime or so, which are being asked to verify
>   against a single specific bank and the result is either yes or no with
>   no inbetween.  In particular, these wouldn't fallback to checking
>   whether something else like padded SHA1s would perhaps verify.
> - The ones more in the development/debugging/testsuite realm, which
>   would attempt to verify against all banks and fail (the test?) if any
>   does not verify. These would try harder to avoid false negatives by
>   testing for the alternatives like padded SHA1s as well. I suppose
>   ima-evm-utils' ima_measurement would qualify as such one.
> 
> For the first class, simply invalidating the unsupported PCR banks
> somehow, i.e. option a.), is good enough.  For the second kind however,
> the question is whether something like b.) would be helpful and the
> additional complexity on the kernel side warranted. But agreed, it's a
> partial and best-effort solution. If one kexecs into a kernel with a
> proper subset of supported TPM bank hashes, it wouldnt't work.

Refer to comment on "Replacing the "Kconfig select".

> 
> > My concern with 'b' is the ability to read the multiple TPM bank PCRs so early
> > during kernel initialization.  Will it succeed?  If it does succeed, will it
> > introduce initialization delays?
> 
> As the boot aggregate gets extended already at that point in time
> (IIRC), I'd expect that reading the PCRs would probably succeed as
> well. For the delays imposed on the kexec restore path -- I can't tell
> unfortunately. But I would only do it on kexec restore if the
> measurement list is non-empty anyway, so systems not having IMA enabled
> or ones which wouldn't kexec are not affected.

Ok
> 
> 
> > FYI, because the IMA Kconfig selects SHA1, we're guaranteed that SHA1 exists in
> > the kernel and the subsequent kexec'ed kernel.  For this reason we're guaranteed
> > that the measurement list is complete.  The simplest solution, not necessarily
> > the best, would be to punt the problem for the time being by replacing the
> > "select" with a different hash algorithm.
> 
> Yes, that would work as well. IIUC, it would mean that we would
> e.g. extend truncated SHA-256 template hashes into a SHA-1 bank, right?
> However, since no existing tool like 'ima_measurement' is expecting
> that, and would fail a verification then, I'm currently struggling to
> see the advantage over just doing a.) and invalidating the PCR banks
> with a fixed value right away?

Replacing the "Kconfig select" has more to do with having at least one
guaranteed complete measurement list.  I'm fine with extending a TPM bank with
an unknown kernel hash algorithm violation (either option a or b).

> 
> > > > This patch set introduces a new definition of integrity violation. Previously it
> > > > was limited to open-writers and ToMToU integrity violations.  Now it could also
> > > > mean no kernel hash algorithm available.  Unfortunately some attestation
> > > > services simply ignore integrity violations.
> > > 
> > > Yeah, there's indeed an ambiguity. I think the right thing to do is to
> > > make measurement lists unverifiable against unsupported banks and would
> > > propose to use 0xfe ... fe for the associated invalidations instead of
> > > the 0xff .. ff used for violation events already.
> > 
> > I just realized that unlike the existing open-writers/ToMToU violations, by
> > definition the new unsupported bank violation would not be included in the
> > measurement list, but just extended into the TPM.
> 
> That's true, but when invalidating unsupported banks with a single ff
> ... ff, one could successfully verify a measurement list having only a
> single violation entry against an invalidated bank AFAICS. I think it
> would be more robust to use a different constant for the invalidation.

Agreed.

thanks,

Mimi

> 
> 
> > > > > So implement d.). As it potentially breaks existing userspace, i.e.
> > > > > the current implementation of ima_measurement, put it behind a Kconfig
> > > > > option, "IMA_COMPAT_FALLBACK_TPM_EXTEND". If set to "y", the original
> > > > > behavior of extending with padded SHA-1 is retained. Otherwise the new
> > > > > scheme to invalidate unsupported PCR banks once upon their first extension
> > > > > from IMA is implemented instead. As ima_measurement is marked as
> > > > > experimental and I find it unlikely that other existing tools depend on
> > > > > the padded SHA-1 fallback scheme, make the IMA_COMPAT_FALLBACK_TPM_EXTEND
> > > > > Kconfig option default to "n".
> > > > > 
> > > > > For IMA_COMPAT_FALLBACK_TPM_EXTEND=n,
> > > > > - make ima_calc_field_array_hash() to fill the digests corresponding to
> > > > >   banks with unsupported hash algorithms with 0xffs,
> > > > > - make ima_pcr_extend() to extend these into the unsupported PCR banks only
> > > > >   upon the PCR's first usage, skip them on subsequent updates and
> > > > > - let ima_init_ima_crypto() help it with that by populating the new
> > > > >   ima_unsupported_tpm_banks_mask with one bit set for each bank with
> > > > >   an unavailable hash algorithm at init.
> > > > > 
> > > > > [1] https://github.com/linux-integrity/ima-evm-utils
> > > > > 
> > > > > Signed-off-by: Nicolai Stange <nstange@suse.de>
> > > > > ---
> > > 
> > 
> 

Re: [RFC PATCH v1 6/7] ima: invalidate unsupported PCR banks once at first use

[Nicolai Stange]

Mimi Zohar <zohar@linux.ibm.com> writes:
> On Tue, 2025-03-18 at 16:55 +0100, Nicolai Stange wrote:
>> Mimi Zohar <zohar@linux.ibm.com> writes:
>> > FYI, because the IMA Kconfig selects SHA1, we're guaranteed that SHA1 exists in
>> > the kernel and the subsequent kexec'ed kernel.  For this reason we're guaranteed
>> > that the measurement list is complete.  The simplest solution, not necessarily
>> > the best, would be to punt the problem for the time being by replacing the
>> > "select" with a different hash algorithm.
>> 
>> Yes, that would work as well. IIUC, it would mean that we would
>> e.g. extend truncated SHA-256 template hashes into a SHA-1 bank, right?
>> However, since no existing tool like 'ima_measurement' is expecting
>> that, and would fail a verification then, I'm currently struggling to
>> see the advantage over just doing a.) and invalidating the PCR banks
>> with a fixed value right away?
>
> Replacing the "Kconfig select" has more to do with having at least one
> guaranteed complete measurement list.  I'm fine with extending a TPM bank with
> an unknown kernel hash algorithm violation (either option a or b).

Ok, I think I got it now.

FWIW, a v2 can be found at
https://lore.kernel.org/r/20250323140911.226137-1-nstange@suse.de , including a
patch for selecting SHA256 now.

Thanks a lot for all your feedback!

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)