[RFC PATCH v2 03/13] ima: invalidate unsupported PCR banks

[Nicolai Stange <nstange@suse.de>]

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.) Don't extend unsupported banks at all.
c.) Record every event as a violation, i.e. extend unsupported banks
    with 0xffs.
d.) Invalidate unsupported banks at least once by extending with a unique
    constant (e.g. with 0xfes).

a.) would make verification from tools like ima_measurement nearly
    impossible, as it would have to guess or somehow determine ima_hash.
b.) is a security risk, because the bank would validate an empty
    measurement list.
c.) isn't ideal security-wise either, because an unsupported bank would
    then validate an all-violations measurement log.
d.) is the only remaining viable option: extending unsupported PCR banks
    at least once with a unique constant of 0xfe ... fe not used for
    anything else would make those not to validate anything from that
    point on.

For this last alternative d.), there are some variations possible, which
differ in the number of times the magic 0xfe ... fe gets extended into
unsupported banks for invalidation purposes. Among the practical ones
are:
- invalidate each unsupported bank over and over again for each new
  measurement log entry or
- invalidate each unsupported bank exactly once.

The second option has the advantage over the first that it would enable
tools like ima-evm-utils' ima_measurement to recognize unsupported banks
in O(1) time, just by comparing the PCR bank value against the constant
HASH(00 .. 00 | fe .. fe). Note that if OTOH a bank got invalidated over
and over again for each single log entry, and assuming that it's desired
to report unsupported banks as such instead of just failing their
validation, ima_measurement would have to try to match yet another PCR
extension candidate sequence for the 0xfe .. fe over the complete
measurement list, as it's currently being done for the padded SHA1s
template hashes already.

As appealing as the scheme to invalidate each unsupported bank exactly once
might seem at first glance though, there's also the clear drawback of an
additional tracking burden with a significant complexity on the kernel
side: because IMA can't know ahead of time which out of all possible PCRs
would ever get referenced from some policy rules, it cannot simply run the
invalidation of unsupported banks upfront at __init, but would have to do
it lazily upon a given PCR's first extension. The need for carrying the
required state across kexecs, with the possibility of different kernels in
the kexec chain potentially supporting different sets of hash algorithms,
doesn't exactly make things easier either.

So, to move towards the original goal of disentangling IMA from its hard
dependency on SHA-1, go with the more straightforward route for now to
invalidate unsupported PCR banks over and over again for each new
measurement list entry recorded. The more sophisticated
"invalidate-exactly-once" scheme will be left to future patches, if to be
implemented at all.

As this 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 by extending with constant 0xfe ... fe
will be effective. 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,
- to cover PCR extensions for "regular" measurement list entries, make
  ima_calc_field_array_hash() to fill the digests corresponding to banks
  with unsupported hash algorithms with 0xfes,
- to cover PCR extensions for violation entries, make ima_init_digest() to
  likewise provision the digests[] elements corresponding to unsupported
  banks with 0xfes.

[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_crypto.c | 19 ++++++++++++++++++-
 security/integrity/ima/ima_queue.c  | 12 ++++++++++++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 475c32615006..c8f12a4a4edf 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 "Enable compatibility TPM PCR extend for unsupported banks"
+	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_crypto.c b/security/integrity/ima/ima_crypto.c
index 6f5696d999d0..a43080fb8edc 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -625,26 +625,43 @@ 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
+		 * 0xfes. This is the value to invalidate unsupported
+		 * PCR banks with. 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, 0xfe, TPM_DIGEST_SIZE);
+#endif
 			continue;
 		}
 
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 83d53824aa98..0cc1189446a8 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -274,11 +274,23 @@ 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;
 
 		memset(digests[i].digest, 0xff, digest_size);
+#else
+		if (ima_algo_array[i].tfm) {
+			memset(digests[i].digest, 0xff, digest_size);
+		} else {
+			/*
+			 * Unsupported banks are invalidated with 0xfe ... fe
+			 * to disambiguate from violations.
+			 */
+			memset(digests[i].digest, 0xfe, digest_size);
+		}
+#endif
 	}
 
 	return 0;
-- 
2.49.0