linux-kernel - [PATCH v2] ima: Suspend PCR extends and log appends when rebooting

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241112165206.756351-1-stefanb@linux.ibm.com>
Date: Tue, 12 Nov 2024 11:52:06 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: linux-integrity@...r.kernel.org, linux-security-module@...r.kernel.org,
        zohar@...ux.ibm.com
Cc: linux-kernel@...r.kernel.org, roberto.sassu@...wei.com,
        Stefan Berger <stefanb@...ux.ibm.com>,
        Tushar Sugandhi <tusharsu@...ux.microsoft.com>
Subject: [PATCH v2] ima: Suspend PCR extends and log appends when rebooting

To avoid the following types of error messages due to a failure by the TPM
driver to use the TPM, suspend TPM PCR extensions and the appending of
entries to the IMA log once IMA's reboot notifier has been called. This
avoids trying to use the TPM after the TPM subsystem has been shut down.

[111707.685315][    T1] ima: Error Communicating to TPM chip, result: -19
[111707.685960][    T1] ima: Error Communicating to TPM chip, result: -19

This error could be observed on a ppc64 machine running SuSE Linux where
processes are still accessing files after devices have been shut down.

Suspending the IMA log and PCR extensions shortly before reboot does not
seem to open a significant measurement gap since neither TPM quoting would
work for attestation nor that new log entries could be written to anywhere
after devices have been shut down. However, there's a time window between
the invocation of the reboot notifier and the shutdown of devices in
kernel_restart_prepare() where __usermodehelper_disable() waits for all
running_helpers to exit. During this time window IMA could now miss log
entries even though attestation would still work. The reboot of the system
shortly after may make this small gap insignificant.

Signed-off-by: Tushar Sugandhi <tusharsu@...ux.microsoft.com>
Signed-off-by: Stefan Berger <stefanb@...ux.ibm.com>

---
 v2:
  - followed Mimi's suggestions

---
 security/integrity/ima/ima.h       |  1 +
 security/integrity/ima/ima_init.c  |  2 ++
 security/integrity/ima/ima_queue.c | 43 ++++++++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3c323ca213d4..3f1a82b7cd71 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -278,6 +278,7 @@ unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
 void ima_init_template_list(void);
 int __init ima_init_digests(void);
+void __init ima_init_reboot_notifier(void);
 int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
 			  void *lsm_data);
 
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 4e208239a40e..a2f34f2d8ad7 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -152,6 +152,8 @@ int __init ima_init(void)
 
 	ima_init_key_queue();
 
+	ima_init_reboot_notifier();
+
 	ima_measure_critical_data("kernel_info", "kernel_version",
 				  UTS_RELEASE, strlen(UTS_RELEASE), false,
 				  NULL, 0);
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 532da87ce519..9b3c9587313f 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -16,6 +16,7 @@
  */
 
 #include <linux/rculist.h>
+#include <linux/reboot.h>
 #include <linux/slab.h>
 #include "ima.h"
 
@@ -44,6 +45,12 @@ struct ima_h_table ima_htable = {
  */
 static DEFINE_MUTEX(ima_extend_list_mutex);
 
+/*
+ * Used internally by the kernel to suspend measurements.
+ * Protected by ima_extend_list_mutex.
+ */
+static bool ima_measurements_suspended;
+
 /* 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)
@@ -176,6 +183,17 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 		}
 	}
 
+	/*
+	 * ima_measurements_suspended will be set before the TPM subsystem has
+	 * been shut down.
+	 */
+	if (ima_measurements_suspended) {
+		audit_cause = "measurements_suspended";
+		audit_info = 0;
+		result = -ENODEV;
+		goto out;
+	}
+
 	result = ima_add_digest_entry(entry,
 				      !IS_ENABLED(CONFIG_IMA_DISABLE_HTABLE));
 	if (result < 0) {
@@ -211,6 +229,31 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)
 	return result;
 }
 
+static void ima_measurements_suspend(void)
+{
+	mutex_lock(&ima_extend_list_mutex);
+	ima_measurements_suspended = true;
+	mutex_unlock(&ima_extend_list_mutex);
+}
+
+static int ima_reboot_notifier(struct notifier_block *nb,
+			       unsigned long action,
+			       void *data)
+{
+	ima_measurements_suspend();
+
+	return NOTIFY_DONE;
+}
+
+static struct notifier_block ima_reboot_nb = {
+	.notifier_call = ima_reboot_notifier,
+};
+
+void __init ima_init_reboot_notifier(void)
+{
+	register_reboot_notifier(&ima_reboot_nb);
+}
+
 int __init ima_init_digests(void)
 {
 	u16 digest_size;
-- 
2.43.0