IMA: make runtime measurement list pollable

PATCH Superseded Daiki Ueno 2 participants 4 messages Oct 5, 2021 – Oct 8, 2021

Subsystems (4)

Security (LSM) Keys-trusted Integrity measurement architecture (IMA) Extended Verification Module (EVM)

lore

Share & tools

lore

Daiki Ueno

author

2021-10-05 14:44 UTC #0 lore raw

PATCH 1/1 IMA: make runtime measurement list pollable

The IMA runtime measurement list exposed on securityfs is currently not pollable, so applications that incrementally verify the measurements have to monitor the file periodically. This patch makes it possible to poll the file in a similar fashion to the sysfs files, i.e., POLLPRI will be signalled on any changes to the list.

Signed-off-by Daiki Ueno <[email protected]>

---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_fs.c    | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_queue.c |  6 ++++++
 3 files changed, 30 insertions(+)

security/integrity/ima/ima.h +2 -0

index be965a8715e4..a7070a74a7ff 100644
@@ -180,6 +180,8 @@ struct ima_h_table {
 };
 extern struct ima_h_table ima_htable;
 
+extern wait_queue_head_t ima_htable_wait;
+
 static inline unsigned int ima_hash_key(u8 *digest)
 {
 	/* there is no point in taking a hash of part of a digest */

security/integrity/ima/ima_fs.c +22 -0

index 3d8e9d5db5aa..f8741784217d 100644
@@ -21,6 +21,7 @@
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/vmalloc.h>
+#include <linux/poll.h>
 
 #include "ima.h"

@@ -198,11 +199,31 @@ static int ima_measurements_open(struct inode *inode, struct file *file)
 	return seq_open(file, &ima_measurments_seqops);
 }
 
+static __poll_t ima_measurements_poll(struct file *file, poll_table *wait)
+{
+	struct seq_file *seq = file->private_data;
+	/* always allow reading */
+	__poll_t mask = EPOLLIN | EPOLLRDNORM;
+	int event;
+
+	poll_wait(file, &ima_htable_wait, wait);
+
+	event = atomic_long_read(&ima_htable.len);
+
+	if (seq->poll_event != event) {
+		seq->poll_event = event;
+		mask |= EPOLLERR | EPOLLPRI;
+	}
+
+	return mask;
+}
+
 static const struct file_operations ima_measurements_ops = {
 	.open = ima_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)

@@ -269,6 +290,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 static ssize_t ima_read_policy(char *path)

security/integrity/ima/ima_queue.c +6 -0

index 532da87ce519..375325dfe449 100644
@@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+/* wait queue for polling changes in ima_htable */
+DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
+
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.

@@ -119,6 +122,9 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
+
+	wake_up_interruptible(&ima_htable_wait);
+
 	return 0;
 }

Signature · 1 line

2.31.1

Mimi Zohar

maintainer

2021-10-06 23:18 UTC #1 lore raw

Hi Daiki,

Quoted 7 lines · Daiki Ueno

The IMA runtime measurement list exposed on securityfs is currently not pollable, so applications that incrementally verify the measurements have to monitor the file periodically. This patch makes it possible to poll the file in a similar fashion to the sysfs files, i.e., POLLPRI will be signalled on any changes to the list.
Signed-off-by Daiki Ueno <[email protected]>

With the sample code in the poll man page and without this patch, the ascii measurement list is pollable. I must be missing something.

Quoted 16 lines

---

security/integrity/ima/ima_queue.c +3 -0

index 532da87ce519..375325dfe449 100644
@@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+/* wait queue for polling changes in ima_htable */
+DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
+
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.

security/integrity/ima/ima_queue.c static int ima_add_digest_entry(struct ima_template_entry *entry, · line 130

Quoted 9 lines

security/integrity/ima/ima_queue.c +3 -0

@@ -119,6 +122,9 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
+
+	wake_up_interruptible(&ima_htable_wait);
+
 	return 0;
 }

The call to wake_up_interruptible() probably should not be here, but in
the caller after the TPM is extended. With that change, you would be
able to differentiate between restoring the measurement list after kexec and regular additional measurements.

thanks,

Mimi

Daiki Ueno

author

2021-10-07 08:38 UTC #2 lore raw

Hello Mimi,

Quoted 9 lines · Mimi Zohar

Hi Daiki,
Quoted 6 lines · Daiki Ueno
The IMA runtime measurement list exposed on securityfs is currently not pollable, so applications that incrementally verify the measurements have to monitor the file periodically. This patch makes it possible to poll the file in a similar fashion to the sysfs files, i.e., POLLPRI will be signalled on any changes to the list.

Quoted 4 lines

Quoted 1 line
Signed-off-by Daiki Ueno <[email protected]>
With the sample code in the poll man page and without this patch, the ascii measurement list is pollable. I must be missing something.

The sample code polls with POLLIN, so every poll() call returns immediately, even if there is no change in the measurement list. That is not ideal in some scenarios[1], and could be a CPU hog as the calling process never sleeps.

With this patch, poll() behaves the same for POLLIN, while the user program is also able to wait for actual changes to the measurement list, with POLLPRI.

Quoted 4 lines

The call to wake_up_interruptible() probably should not be here, but in
the caller after the TPM is extended. With that change, you would be
able to differentiate between restoring the measurement list after kexec and regular additional measurements.

Thank you for the suggestion; I will test that change and get it back to you soon.

Regards,

Footnotes:
[1] https://github.com/keylime/keylime/blob/master/keylime/cmd/ima_emulator_adapter.py#L95
Currently the poll() in the loop is no-op and it would spin if
there is no time.sleep()

Signature · 1 line

Daiki Ueno

Daiki Ueno

author

2021-10-08 06:54 UTC #3 lore raw

PATCH v2 1/1 IMA: make runtime measurement list pollable

In reply to Daiki Ueno

Currently it's not possible to efficiently monitor changes to the IMA runtime measurement list with poll(). While ascii_runtime_measurement file is pollable with POLLIN, it returns immediately even if there is no change to the list, and thus cannot be used as a wait condition in a loop.

This patch makes it possible to poll the file in a similar fashion to the sysfs files: POLLIN is still always signalled, while POLLPRI will also be signalled on actual changes made to the list.

Signed-off-by Daiki Ueno <[email protected]>

---
v2: moved wake_up_interruptible() call after PCR extend and expanded the patch description to cover the use-case, suggested by Mimi.

---
 security/integrity/ima/ima.h       |  2 ++
 security/integrity/ima/ima_fs.c    | 22 ++++++++++++++++++++++
 security/integrity/ima/ima_queue.c |  8 +++++++-
 3 files changed, 31 insertions(+), 1 deletion(-)

security/integrity/ima/ima.h +2 -0

index be965a8715e4..a7070a74a7ff 100644
@@ -180,6 +180,8 @@ struct ima_h_table {
 };
 extern struct ima_h_table ima_htable;
 
+extern wait_queue_head_t ima_htable_wait;
+
 static inline unsigned int ima_hash_key(u8 *digest)
 {
 	/* there is no point in taking a hash of part of a digest */

security/integrity/ima/ima_fs.c +22 -0

index 3d8e9d5db5aa..f8741784217d 100644
@@ -21,6 +21,7 @@
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/vmalloc.h>
+#include <linux/poll.h>
 
 #include "ima.h"

@@ -198,11 +199,31 @@ static int ima_measurements_open(struct inode *inode, struct file *file)
 	return seq_open(file, &ima_measurments_seqops);
 }
 
+static __poll_t ima_measurements_poll(struct file *file, poll_table *wait)
+{
+	struct seq_file *seq = file->private_data;
+	/* always allow reading */
+	__poll_t mask = EPOLLIN | EPOLLRDNORM;
+	int event;
+
+	poll_wait(file, &ima_htable_wait, wait);
+
+	event = atomic_long_read(&ima_htable.len);
+
+	if (seq->poll_event != event) {
+		seq->poll_event = event;
+		mask |= EPOLLERR | EPOLLPRI;
+	}
+
+	return mask;
+}
+
 static const struct file_operations ima_measurements_ops = {
 	.open = ima_measurements_open,
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size)

@@ -269,6 +290,7 @@ static const struct file_operations ima_ascii_measurements_ops = {
 	.read = seq_read,
 	.llseek = seq_lseek,
 	.release = seq_release,
+	.poll = ima_measurements_poll,
 };
 
 static ssize_t ima_read_policy(char *path)

security/integrity/ima/ima_queue.c +7 -1

index 532da87ce519..de4941909a93 100644
@@ -38,6 +38,9 @@ struct ima_h_table ima_htable = {
 	.queue[0 ... IMA_MEASURE_HTABLE_SIZE - 1] = HLIST_HEAD_INIT
 };
 
+/* wait queue for polling changes in ima_htable */
+DECLARE_WAIT_QUEUE_HEAD(ima_htable_wait);
+
 /* mutex protects atomicity of extending measurement list
  * and extending the TPM PCR aggregate. Since tpm_extend can take
  * long (and the tpm driver uses a mutex), we can't use the spinlock.

@@ -119,6 +122,7 @@ static int ima_add_digest_entry(struct ima_template_entry *entry,
 		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
 		     binary_runtime_size + size : ULONG_MAX;
 	}
+
 	return 0;
 }

@@ -193,9 +197,11 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			 tpmresult);
 		audit_cause = tpm_audit_cause;
 		audit_info = 0;
-	}
+	} else
+		wake_up_interruptible(&ima_htable_wait);
 out:
 	mutex_unlock(&ima_extend_list_mutex);
+
 	integrity_audit_msg(AUDIT_INTEGRITY_PCR, inode, filename,
 			    op, audit_cause, result, audit_info);
 	return result;

Signature · 1 line

2.31.1