Message-ID: <20250313173339.3815589-2-nstange@suse.de>
Date: Thu, 13 Mar 2025 18:33:33 +0100
From: Nicolai Stange <nstange@...e.de>
To: Mimi Zohar <zohar@...ux.ibm.com>,
	Roberto Sassu <roberto.sassu@...wei.com>,
	Dmitry Kasatkin <dmitry.kasatkin@...il.com>
Cc: Eric Snowberg <eric.snowberg@...cle.com>,
	linux-integrity@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Nicolai Stange <nstange@...e.de>
Subject: [RFC PATCH v1 1/7] ima: don't expose runtime_measurements for unsupported hashes

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@...e.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

Powered by blists - more mailing lists