linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC] Persist ima logs to disk
@ 2021-01-05 19:57 Raphael Gianotti
  2021-01-07 15:06 ` Mimi Zohar
  2021-01-07 15:45 ` Janne Karhunen
  0 siblings, 2 replies; 23+ messages in thread
From: Raphael Gianotti @ 2021-01-05 19:57 UTC (permalink / raw)
  To: zohar, janne.karhunen; +Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib

IMA measures files and buffer data and some systems may end up
generating lots of entries in the IMA measurement list. This list is
kept in kernel memoryc and as it grows in size it could end up taking
too many resources, causing the system to run out of available
memory. During kexec, the IMA measurement list can be carried over in
memory, but it's possible for the list to become too large for that
to happen.

The Kconfig introduced in this series enables admins to configure a
maximum number of entries and a file to export the IMA measurement
list to whenever the set limit is reached.

The list is written out in append mode, so the system will keep
writing new entries as long as it stays running or runs out of
space. Whenever the export file is set, it's truncated. If writing
to the export list fails, a flag is set to prevent further exports,
as the file is likely in a bad state. Setting a new export file
resets this flag, allowing exports to resume and giving admins a way
to recover from this state if necessary.

In the case of kexec, if the list is too large too be carried over in
memory and an export file is configured, the list will be exported,
preventing the measurements from being lost during kexec.

This code is based off of a previous RFC sent by Janne Karhunen[1],
and is intended to pick up where that was left off.

In a thread with Janne Karhunen[2], it was mentioned that another
approach, using mm had been considered. Upon some investigation the
approach used in this RFC still seemed adequate for solving this
problem.

[1] https://patchwork.kernel.org/project/linux-integrity/patch/201912
20074929.8191-1-janne.karhunen@gmail.com/
[2] https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/

Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
---
 security/integrity/ima/Kconfig     |   9 ++
 security/integrity/ima/ima.h       |   7 ++
 security/integrity/ima/ima_api.c   |   1 +
 security/integrity/ima/ima_fs.c    | 194 +++++++++++++++++++++++++++++
 security/integrity/ima/ima_kexec.c |   7 +-
 security/integrity/ima/ima_queue.c |   2 +-
 6 files changed, 217 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 12e9250c1bec..faea01fc1dd1 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -334,3 +334,12 @@ config IMA_SECURE_AND_OR_TRUSTED_BOOT
        help
           This option is selected by architectures to enable secure and/or
           trusted boot based on IMA runtime policies.
+config IMA_MEASUREMENT_ENTRY_COUNT
+	int
+	depends on IMA
+	default 2000
+	help
+	   This option defines the maximum number of entries in the
+	   measurement list. Once the limit is reached, the entire
+	   list is exported to a user defined file and the kernel
+	   list is freed.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 38043074ce5e..d072943149d8 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -47,6 +47,9 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8, TPM_PCR10 = 10 };
 
 #define NR_BANKS(chip) ((chip != NULL) ? chip->nr_allocated_banks : 0)
 
+#define secfs_mnt       "/sys/kernel/security"
+#define am_filename     "/integrity/ima/ascii_runtime_measurements"
+
 /* current content of the policy */
 extern int ima_policy_flag;
 
@@ -158,6 +161,7 @@ int template_desc_init_fields(const char *template_fmt,
 struct ima_template_desc *ima_template_desc_current(void);
 struct ima_template_desc *lookup_template_desc(const char *name);
 bool ima_template_has_modsig(const struct ima_template_desc *ima_template);
+void ima_free_template_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
@@ -167,12 +171,15 @@ void ima_init_template_list(void);
 int __init ima_init_digests(void);
 int ima_lsm_policy_change(struct notifier_block *nb, unsigned long event,
 			  void *lsm_data);
+int ima_export_list(const char *sourcefile);
 
 /*
  * used to protect h_table and sha_table
  */
 extern spinlock_t ima_queue_lock;
 
+extern struct mutex ima_extend_list_mutex;
+
 struct ima_h_table {
 	atomic_long_t len;	/* number of stored measurements in the list */
 	atomic_long_t violations;
diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c
index 4f39fb93f278..37fbf59547c1 100644
--- a/security/integrity/ima/ima_api.c
+++ b/security/integrity/ima/ima_api.c
@@ -120,6 +120,7 @@ int ima_store_template(struct ima_template_entry *entry,
 	}
 	entry->pcr = pcr;
 	result = ima_add_template_entry(entry, violation, op, inode, filename);
+	ima_export_list(NULL);
 	return result;
 }
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e3fcad871861..dd9e2d04e9bc 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -20,10 +20,15 @@
 #include <linux/rcupdate.h>
 #include <linux/parser.h>
 #include <linux/vmalloc.h>
+#include <linux/fs_struct.h>
+#include <linux/syscalls.h>
+#include <../fs/internal.h>
 
 #include "ima.h"
 
 static DEFINE_MUTEX(ima_write_mutex);
+static DEFINE_MUTEX(ima_list_mutex);
+static char *ima_msmt_list_name = NULL;
 
 bool ima_canonical_fmt;
 static int __init default_canonical_fmt_setup(char *str)
@@ -37,6 +42,9 @@ __setup("ima_canonical_fmt", default_canonical_fmt_setup);
 
 static int valid_policy = 1;
 
+static bool init_list_export = true;
+static bool list_export_ok = true;
+
 static ssize_t ima_show_htable_value(char __user *buf, size_t count,
 				     loff_t *ppos, atomic_long_t *val)
 {
@@ -359,6 +367,7 @@ static struct dentry *ascii_runtime_measurements;
 static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
+static struct dentry *ima_list_name;
 
 enum ima_fs_flags {
 	IMA_FS_BUSY,
@@ -446,6 +455,184 @@ static const struct file_operations ima_measure_policy_ops = {
 	.llseek = generic_file_llseek,
 };
 
+/*
+ * (Called with ima_extend_list_mutex held.)
+ */
+static void ima_free_list(void)
+{
+	struct ima_queue_entry *qe, *e;
+
+	list_for_each_entry_safe(qe, e, &ima_measurements, later) {
+		hlist_del_rcu(&qe->hnext);
+		list_del_rcu(&qe->later);
+		ima_free_template_entry(qe->entry);
+		kzfree(qe);
+	}
+	atomic_long_set(&ima_htable.len, 0);
+}
+
+static int ima_unlink_file(const char *filename)
+{
+	struct filename *file;
+
+	file = getname_kernel(filename);
+	if (IS_ERR(file))
+		return -EINVAL;
+	
+	return do_unlinkat(AT_FDCWD, file);
+}
+
+int ima_export_list(const char *sourcefile)
+{
+	struct file *file_out = NULL;
+	struct file *file_in = NULL;
+	const char *targetfile = ima_msmt_list_name;
+	ssize_t bytesin, bytesout;
+	mm_segment_t fs;
+	struct path root;
+	loff_t offin = 0, offout = 0;
+	char data[512];
+	long htable_len;
+	int err = 0;
+
+	htable_len = atomic_long_read(&ima_htable.len);
+	if (CONFIG_IMA_MEASUREMENT_ENTRY_COUNT <= 0)
+		goto out_err;
+	if (sourcefile == NULL && htable_len <= CONFIG_IMA_MEASUREMENT_ENTRY_COUNT)
+		goto out_err;
+	if (targetfile == NULL)
+		goto out_err;
+	if (sourcefile == NULL) {
+		pr_info("msmt list size (%ld/%ld) exceeded, exporting to %s\n",
+                	htable_len, (long)CONFIG_IMA_MEASUREMENT_ENTRY_COUNT, targetfile);
+		sourcefile = secfs_mnt am_filename;
+	}
+	if (!list_export_ok) {
+		err = -EFAULT;
+		goto out_err;
+	}
+
+	fs = get_fs();
+	set_fs(KERNEL_DS);
+
+	if (init_list_export) {
+		ima_unlink_file(targetfile);
+		init_list_export = false;
+	}
+	task_lock(&init_task);
+	get_fs_root(init_task.fs, &root);
+	task_unlock(&init_task);
+
+	file_out = file_open_root(root.dentry, root.mnt, targetfile,
+				  O_CREAT|O_WRONLY|O_APPEND|O_NOFOLLOW,
+				  0400);
+	if (IS_ERR(file_out)) {
+		err = PTR_ERR(file_out);
+		pr_err("failed to open %s, err %d\n", targetfile, err);
+		file_out = NULL;
+		goto out_close;
+	}
+	file_in = file_open_root(root.dentry, root.mnt, sourcefile, O_RDONLY, 0);
+	if (IS_ERR(file_in)) {
+		err = PTR_ERR(file_in);
+		pr_err("failed to open %s, err %d\n", sourcefile, err);
+		file_in = NULL;
+		goto out_close;
+	}
+	mutex_lock(&ima_extend_list_mutex);
+	/*
+	 * if we fail writing, the recovery is a job for the admin.
+	 * Logs will be kept in memory.
+	 */
+	list_export_ok = false;
+	do {
+		bytesin = vfs_read(file_in, data, 512, &offin);
+		if (bytesin < 0) {
+			pr_err("read error at %lld\n", offin);
+			err = -EIO;
+			goto out_unlock;
+		}
+		bytesout = vfs_write(file_out, data, bytesin, &offout);
+		if (bytesout < 0) {
+			pr_err("write error at %lld\n", offout);
+			err = -EIO;
+			goto out_unlock;
+		}
+		if (bytesin != bytesout) {
+			err = bytesout;
+			goto out_unlock;
+		}
+	} while (bytesin == 512);
+	list_export_ok = true;
+	ima_free_list();
+
+out_unlock:
+	if (!list_export_ok) {
+		pr_err("failed to export measurement list to %s", targetfile);
+	}
+	mutex_unlock(&ima_extend_list_mutex);
+out_close:
+	if (file_in)
+		filp_close(file_in, NULL);
+	if (file_out)
+		filp_close(file_out, NULL);
+
+	path_put(&root);
+	set_fs(fs);
+out_err:
+	return err;
+}
+
+static ssize_t ima_write_list_name(struct file *file,
+				   const char __user *buf,
+				   size_t datalen, loff_t *ppos)
+{
+	int err;
+
+	if (!capable(CAP_SYS_ADMIN))
+		return -EPERM;
+
+	if ((datalen <= 1) || (datalen >= 255))
+		return -EINVAL;
+
+	mutex_lock(&ima_list_mutex);
+	
+	ima_msmt_list_name = memdup_user(buf, datalen);
+
+	if (IS_ERR(ima_msmt_list_name)) {
+		err = PTR_ERR(ima_msmt_list_name);
+		goto out_unlock;
+	}
+
+	if (*ima_msmt_list_name != '/') {
+		kfree(ima_msmt_list_name);
+		ima_msmt_list_name = NULL;
+		err = -EINVAL;
+		goto out_unlock;
+	}
+
+	if (ima_msmt_list_name[datalen-1] != '\0')
+		ima_msmt_list_name[datalen-1] = '\0';
+
+	list_export_ok = true;
+	init_list_export = true;
+	err = ima_export_list(NULL);
+	if (err) {
+		pr_err("list export failed with %d\n", err);
+		goto out_unlock;
+	}
+
+	err = datalen;
+
+out_unlock:
+	mutex_unlock(&ima_list_mutex);
+	return err;
+}
+
+static const struct file_operations ima_list_export_ops = {
+	.write = ima_write_list_name,
+};
+
 int __init ima_fs_init(void)
 {
 	ima_dir = securityfs_create_dir("ima", integrity_dir);
@@ -490,6 +677,11 @@ int __init ima_fs_init(void)
 	if (IS_ERR(ima_policy))
 		goto out;
 
+	ima_list_name = securityfs_create_file("runtime_measurements_export_list_path", S_IWUSR | S_IWGRP, ima_dir,
+					       NULL, &ima_list_export_ops);
+	if (IS_ERR(ima_list_name))
+		goto out;
+
 	return 0;
 out:
 	securityfs_remove(violations);
@@ -499,5 +691,7 @@ int __init ima_fs_init(void)
 	securityfs_remove(ima_symlink);
 	securityfs_remove(ima_dir);
 	securityfs_remove(ima_policy);
+	securityfs_remove(ima_list_name);
+
 	return -1;
 }
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..aa4bd4ba07c7 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -102,8 +102,11 @@ void ima_add_kexec_buffer(struct kimage *image)
 					   PAGE_SIZE / 2, PAGE_SIZE);
 	if ((kexec_segment_size == ULONG_MAX) ||
 	    ((kexec_segment_size >> PAGE_SHIFT) > totalram_pages() / 2)) {
-		pr_err("Binary measurement list too large.\n");
-		return;
+		ret = export_ima_list(secfs_mnt am_filename);
+		if (ret) {
+			pr_err("Binary measurement list too large.\n");
+			return;
+		}
 	}
 
 	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index c096ef8945c7..deaea4780359 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -42,7 +42,7 @@ struct ima_h_table ima_htable = {
  * 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.
  */
-static DEFINE_MUTEX(ima_extend_list_mutex);
+DEFINE_MUTEX(ima_extend_list_mutex);
 
 /* 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,
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-05 19:57 [RFC] Persist ima logs to disk Raphael Gianotti
@ 2021-01-07 15:06 ` Mimi Zohar
  2021-01-07 16:42   ` James Bottomley
  2021-01-07 18:32   ` Raphael Gianotti
  2021-01-07 15:45 ` Janne Karhunen
  1 sibling, 2 replies; 23+ messages in thread
From: Mimi Zohar @ 2021-01-07 15:06 UTC (permalink / raw)
  To: Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

[Cc: Amir Goldstein]

On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
> IMA measures files and buffer data and some systems may end up
> generating lots of entries in the IMA measurement list. This list is
> kept in kernel memoryc and as it grows in size it could end up taking
> too many resources, causing the system to run out of available
> memory. During kexec, the IMA measurement list can be carried over in
> memory, but it's possible for the list to become too large for that
> to happen.
> 
> The Kconfig introduced in this series enables admins to configure a
> maximum number of entries and a file to export the IMA measurement
> list to whenever the set limit is reached.
> 
> The list is written out in append mode, so the system will keep
> writing new entries as long as it stays running or runs out of
> space. Whenever the export file is set, it's truncated. If writing
> to the export list fails, a flag is set to prevent further exports,
> as the file is likely in a bad state. Setting a new export file
> resets this flag, allowing exports to resume and giving admins a way
> to recover from this state if necessary.
> 
> In the case of kexec, if the list is too large too be carried over in
> memory and an export file is configured, the list will be exported,
> preventing the measurements from being lost during kexec.
> 
> This code is based off of a previous RFC sent by Janne Karhunen[1],
> and is intended to pick up where that was left off.
> 
> In a thread with Janne Karhunen[2], it was mentioned that another
> approach, using mm had been considered. Upon some investigation the
> approach used in this RFC still seemed adequate for solving this
> problem.
> 
> [1] https://patchwork.kernel.org/project/linux-integrity/patch/201912
> 20074929.8191-1-janne.karhunen@gmail.com/
> [2] https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
> vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
> 
> Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>

My original concerns of truncating the IMA measurement list have not
been addressed.  Once the IMA measurement list has been truncated,
quoting and then verifying any of the PCRs contained in the measurement
list will fail, unless the measurements have been preserved and are
readily accessible.

Amir's suggestion addresses kernel memory constraints without
truncating the IMA measurement list.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-05 19:57 [RFC] Persist ima logs to disk Raphael Gianotti
  2021-01-07 15:06 ` Mimi Zohar
@ 2021-01-07 15:45 ` Janne Karhunen
  2021-01-07 18:35   ` Raphael Gianotti
  1 sibling, 1 reply; 23+ messages in thread
From: Janne Karhunen @ 2021-01-07 15:45 UTC (permalink / raw)
  To: Raphael Gianotti
  Cc: Mimi Zohar, linux-integrity, tusharsu, tyhicks,
	Lakshmi Ramasubramanian, balajib

On Tue, Jan 5, 2021 at 9:57 PM Raphael Gianotti
<raphgi@linux.microsoft.com> wrote:

> In a thread with Janne Karhunen[2], it was mentioned that another
> approach, using mm had been considered. Upon some investigation the
> approach used in this RFC still seemed adequate for solving this
> problem.

Curious to hear in more detail where did this land?

Not sure I remember this correctly anymore, but wouldn't it be
possible to have mmap'd tmpfile at address __heap of size __heap_sz
and have something this simple pulling memory from it?

uint8_t *get_static_buffer(size_t size)
{
        static size_t buf_index;
        uint8_t *bufp = NULL;

        size = ROUND_UP(size, sizeof(double));
        if ((buf_index + size) >= __heap_sz)
                return NULL;

        bufp = (uint8_t *)__heap + buf_index;
        buf_index += size;
        *bufp = 0;

        return bufp;
}

Then just replace every related measurement list allocation with this
get_static_buffer and that would be pretty much all there is to it?

The mm code should automatically push those pages out when it needs
memory. It will also read those pages back in when someone scans
through the measurement list creating a nicely formatted one. I think.


--
Janne

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 15:06 ` Mimi Zohar
@ 2021-01-07 16:42   ` James Bottomley
  2021-01-07 20:02     ` Mimi Zohar
  2021-01-07 18:32   ` Raphael Gianotti
  1 sibling, 1 reply; 23+ messages in thread
From: James Bottomley @ 2021-01-07 16:42 UTC (permalink / raw)
  To: Mimi Zohar, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 10:06 -0500, Mimi Zohar wrote:
> [Cc: Amir Goldstein]
> 
> On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
> > IMA measures files and buffer data and some systems may end up
> > generating lots of entries in the IMA measurement list. This list
> > is kept in kernel memoryc and as it grows in size it could end up
> > taking too many resources, causing the system to run out of
> > available memory. During kexec, the IMA measurement list can be
> > carried over in memory, but it's possible for the list to become
> > too large for that to happen.
> > 
> > The Kconfig introduced in this series enables admins to configure a
> > maximum number of entries and a file to export the IMA measurement
> > list to whenever the set limit is reached.
> > 
> > The list is written out in append mode, so the system will keep
> > writing new entries as long as it stays running or runs out of
> > space. Whenever the export file is set, it's truncated. If writing
> > to the export list fails, a flag is set to prevent further exports,
> > as the file is likely in a bad state. Setting a new export file
> > resets this flag, allowing exports to resume and giving admins a
> > way to recover from this state if necessary.
> > 
> > In the case of kexec, if the list is too large too be carried over
> > in memory and an export file is configured, the list will be
> > exported, preventing the measurements from being lost during kexec.
> > 
> > This code is based off of a previous RFC sent by Janne Karhunen[1],
> > and is intended to pick up where that was left off.
> > 
> > In a thread with Janne Karhunen[2], it was mentioned that another
> > approach, using mm had been considered. Upon some investigation the
> > approach used in this RFC still seemed adequate for solving this
> > problem.
> > 
> > [1] 
> > https://patchwork.kernel.org/project/linux-integrity/patch/201912
> > 20074929.8191-1-janne.karhunen@gmail.com/
> > [2] 
> > https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
> > vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
> > 
> > Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
> 
> My original concerns of truncating the IMA measurement list have not
> been addressed.  Once the IMA measurement list has been truncated,
> quoting and then verifying any of the PCRs contained in the
> measurement list will fail, unless the measurements have been
> preserved and are readily accessible.
> 
> Amir's suggestion addresses kernel memory constraints without
> truncating the IMA measurement list.

What about having a log entry that's the current PCR value?  Then
stretches of the log starting with these entries would be independently
verifiable provided you had a way of trusting the PCR value.  It might
be possible to get the TPM to add a signed quote as an optional part of
the log entry (of course this brings other problems like which key do
you use for the signing and how does it get verified) which would
provide the trust and would definitively allow you to archive log
segments and still make the rest of the log useful.

James



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 15:06 ` Mimi Zohar
  2021-01-07 16:42   ` James Bottomley
@ 2021-01-07 18:32   ` Raphael Gianotti
  1 sibling, 0 replies; 23+ messages in thread
From: Raphael Gianotti @ 2021-01-07 18:32 UTC (permalink / raw)
  To: Mimi Zohar, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein


On 1/7/2021 7:06 AM, Mimi Zohar wrote:
> [Cc: Amir Goldstein]
>
> On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
>> IMA measures files and buffer data and some systems may end up
>> generating lots of entries in the IMA measurement list. This list is
>> kept in kernel memoryc and as it grows in size it could end up taking
>> too many resources, causing the system to run out of available
>> memory. During kexec, the IMA measurement list can be carried over in
>> memory, but it's possible for the list to become too large for that
>> to happen.
>>
>> The Kconfig introduced in this series enables admins to configure a
>> maximum number of entries and a file to export the IMA measurement
>> list to whenever the set limit is reached.
>>
>> The list is written out in append mode, so the system will keep
>> writing new entries as long as it stays running or runs out of
>> space. Whenever the export file is set, it's truncated. If writing
>> to the export list fails, a flag is set to prevent further exports,
>> as the file is likely in a bad state. Setting a new export file
>> resets this flag, allowing exports to resume and giving admins a way
>> to recover from this state if necessary.
>>
>> In the case of kexec, if the list is too large too be carried over in
>> memory and an export file is configured, the list will be exported,
>> preventing the measurements from being lost during kexec.
>>
>> This code is based off of a previous RFC sent by Janne Karhunen[1],
>> and is intended to pick up where that was left off.
>>
>> In a thread with Janne Karhunen[2], it was mentioned that another
>> approach, using mm had been considered. Upon some investigation the
>> approach used in this RFC still seemed adequate for solving this
>> problem.
>>
>> [1] https://patchwork.kernel.org/project/linux-integrity/patch/201912
>> 20074929.8191-1-janne.karhunen@gmail.com/
>> [2] https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
>> vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
>>
>> Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
> My original concerns of truncating the IMA measurement list have not
> been addressed.  Once the IMA measurement list has been truncated,
> quoting and then verifying any of the PCRs contained in the measurement
> list will fail, unless the measurements have been preserved and are
> readily accessible.
>
> Amir's suggestion addresses kernel memory constraints without
> truncating the IMA measurement list.
>
> Mimi
I actually wasn't aware of what the exact concerns were since I couldn't 
find the
original discussion around it, I only knew that had been suggested and 
thought
it was mostly due to it being a more elegant solution. In my in looking 
into it I might
have overcomplicated, from looking at a reply from Janne. I will revisit 
that with this
in mind as it seems then this solution won't be able to address the PCR 
concerns.

Raph


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 15:45 ` Janne Karhunen
@ 2021-01-07 18:35   ` Raphael Gianotti
  0 siblings, 0 replies; 23+ messages in thread
From: Raphael Gianotti @ 2021-01-07 18:35 UTC (permalink / raw)
  To: Janne Karhunen
  Cc: Mimi Zohar, linux-integrity, tusharsu, tyhicks,
	Lakshmi Ramasubramanian, balajib


On 1/7/2021 7:45 AM, Janne Karhunen wrote:
> On Tue, Jan 5, 2021 at 9:57 PM Raphael Gianotti
> <raphgi@linux.microsoft.com> wrote:
>
>> In a thread with Janne Karhunen[2], it was mentioned that another
>> approach, using mm had been considered. Upon some investigation the
>> approach used in this RFC still seemed adequate for solving this
>> problem.
> Curious to hear in more detail where did this land?
>
> Not sure I remember this correctly anymore, but wouldn't it be
> possible to have mmap'd tmpfile at address __heap of size __heap_sz
> and have something this simple pulling memory from it?
>
> uint8_t *get_static_buffer(size_t size)
> {
>          static size_t buf_index;
>          uint8_t *bufp = NULL;
>
>          size = ROUND_UP(size, sizeof(double));
>          if ((buf_index + size) >= __heap_sz)
>                  return NULL;
>
>          bufp = (uint8_t *)__heap + buf_index;
>          buf_index += size;
>          *bufp = 0;
>
>          return bufp;
> }
>
> Then just replace every related measurement list allocation with this
> get_static_buffer and that would be pretty much all there is to it?
>
> The mm code should automatically push those pages out when it needs
> memory. It will also read those pages back in when someone scans
> through the measurement list creating a nicely formatted one. I think.
>
>
> --
> Janne
As I replied to the other comment from Mimi, I think I may have been 
overcomplicating
it. My knowledge of mm is still not very deep, so I may have overthought 
some parts of
it and thought anything I was coming up with didn't seem like a good way 
to do it. I will
revisit this, especially since I wasn't aware of the PCR concerns Mimi 
had, which make
sense and will probably be hard to solve with the code I just sent.

Raph


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 16:42   ` James Bottomley
@ 2021-01-07 20:02     ` Mimi Zohar
  2021-01-07 20:37       ` James Bottomley
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-01-07 20:02 UTC (permalink / raw)
  To: James Bottomley, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
> On Thu, 2021-01-07 at 10:06 -0500, Mimi Zohar wrote:
> > [Cc: Amir Goldstein]
> > 
> > On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
> > > IMA measures files and buffer data and some systems may end up
> > > generating lots of entries in the IMA measurement list. This list
> > > is kept in kernel memoryc and as it grows in size it could end up
> > > taking too many resources, causing the system to run out of
> > > available memory. During kexec, the IMA measurement list can be
> > > carried over in memory, but it's possible for the list to become
> > > too large for that to happen.
> > > 
> > > The Kconfig introduced in this series enables admins to configure a
> > > maximum number of entries and a file to export the IMA measurement
> > > list to whenever the set limit is reached.
> > > 
> > > The list is written out in append mode, so the system will keep
> > > writing new entries as long as it stays running or runs out of
> > > space. Whenever the export file is set, it's truncated. If writing
> > > to the export list fails, a flag is set to prevent further exports,
> > > as the file is likely in a bad state. Setting a new export file
> > > resets this flag, allowing exports to resume and giving admins a
> > > way to recover from this state if necessary.
> > > 
> > > In the case of kexec, if the list is too large too be carried over
> > > in memory and an export file is configured, the list will be
> > > exported, preventing the measurements from being lost during kexec.
> > > 
> > > This code is based off of a previous RFC sent by Janne Karhunen[1],
> > > and is intended to pick up where that was left off.
> > > 
> > > In a thread with Janne Karhunen[2], it was mentioned that another
> > > approach, using mm had been considered. Upon some investigation the
> > > approach used in this RFC still seemed adequate for solving this
> > > problem.
> > > 
> > > [1] 
> > > https://patchwork.kernel.org/project/linux-integrity/patch/201912
> > > 20074929.8191-1-janne.karhunen@gmail.com/
> > > [2] 
> > > https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
> > > vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
> > > 
> > > Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
> > 
> > My original concerns of truncating the IMA measurement list have not
> > been addressed.  Once the IMA measurement list has been truncated,
> > quoting and then verifying any of the PCRs contained in the
> > measurement list will fail, unless the measurements have been
> > preserved and are readily accessible.
> > 
> > Amir's suggestion addresses kernel memory constraints without
> > truncating the IMA measurement list.
> 
> What about having a log entry that's the current PCR value?  Then
> stretches of the log starting with these entries would be independently
> verifiable provided you had a way of trusting the PCR value.  It might
> be possible to get the TPM to add a signed quote as an optional part of
> the log entry (of course this brings other problems like which key do
> you use for the signing and how does it get verified) which would
> provide the trust and would definitively allow you to archive log
> segments and still make the rest of the log useful.

The current PCR values are aggregated and stored in the boot_aggregate
record.  As part of the new boot_aggregate record format, the
individual PCR values could be included.

But this doesn't address where the offloaded measurement list will be
stored, how long the list will be retained, nor who guarantees the
integrity of the offloaded list.  In addition, different form factors
will have different requirements.

Mimi




^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 20:02     ` Mimi Zohar
@ 2021-01-07 20:37       ` James Bottomley
  2021-01-07 20:51         ` Mimi Zohar
  0 siblings, 1 reply; 23+ messages in thread
From: James Bottomley @ 2021-01-07 20:37 UTC (permalink / raw)
  To: Mimi Zohar, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 15:02 -0500, Mimi Zohar wrote:
> On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
> > On Thu, 2021-01-07 at 10:06 -0500, Mimi Zohar wrote:
> > > [Cc: Amir Goldstein]
> > > 
> > > On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
> > > > IMA measures files and buffer data and some systems may end up
> > > > generating lots of entries in the IMA measurement list. This
> > > > list is kept in kernel memoryc and as it grows in size it could
> > > > end up taking too many resources, causing the system to run out
> > > > of available memory. During kexec, the IMA measurement list can
> > > > be carried over in memory, but it's possible for the list to
> > > > become too large for that to happen.
> > > > 
> > > > The Kconfig introduced in this series enables admins to
> > > > configure a maximum number of entries and a file to export the
> > > > IMA measurement list to whenever the set limit is reached.
> > > > 
> > > > The list is written out in append mode, so the system will keep
> > > > writing new entries as long as it stays running or runs out of
> > > > space. Whenever the export file is set, it's truncated. If
> > > > writing to the export list fails, a flag is set to prevent
> > > > further exports, as the file is likely in a bad state. Setting
> > > > a new export file resets this flag, allowing exports to resume
> > > > and giving admins a way to recover from this state if
> > > > necessary.
> > > > 
> > > > In the case of kexec, if the list is too large too be carried
> > > > over in memory and an export file is configured, the list will
> > > > be exported, preventing the measurements from being lost during
> > > > kexec.
> > > > 
> > > > This code is based off of a previous RFC sent by Janne
> > > > Karhunen[1], and is intended to pick up where that was left
> > > > off.
> > > > 
> > > > In a thread with Janne Karhunen[2], it was mentioned that
> > > > another approach, using mm had been considered. Upon some
> > > > investigation the approach used in this RFC still seemed
> > > > adequate for solving this problem.
> > > > 
> > > > [1] 
> > > > https://patchwork.kernel.org/project/linux-integrity/patch/201912
> > > > 20074929.8191-1-janne.karhunen@gmail.com/
> > > > [2] 
> > > > https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
> > > > vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
> > > > 
> > > > Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
> > > 
> > > My original concerns of truncating the IMA measurement list have
> > > not been addressed.  Once the IMA measurement list has been
> > > truncated, quoting and then verifying any of the PCRs contained
> > > in the measurement list will fail, unless the measurements have
> > > been preserved and are readily accessible.
> > > 
> > > Amir's suggestion addresses kernel memory constraints without
> > > truncating the IMA measurement list.
> > 
> > What about having a log entry that's the current PCR value?  Then
> > stretches of the log starting with these entries would be
> > independently verifiable provided you had a way of trusting the PCR
> > value.  It might be possible to get the TPM to add a signed quote
> > as an optional part of the log entry (of course this brings other
> > problems like which key do you use for the signing and how does it
> > get verified) which would provide the trust and would definitively
> > allow you to archive log segments and still make the rest of the
> > log useful.
> 
> The current PCR values are aggregated and stored in the
> boot_aggregate record.  As part of the new boot_aggregate record
> format, the individual PCR values could be included.

I don't think we care about the boot aggregate ... it's just the
initial log entry that ties the boot state to the initial runtime
state.  All we need for the proposed entry is the current value of the
IMA PCR so provided you trust that value it becomes a base on which the
following measurements can build and be trusted.

> But this doesn't address where the offloaded measurement list will be
> stored, how long the list will be retained, nor who guarantees the
> integrity of the offloaded list.  In addition, different form factors
> will have different requirements.

I'm not sure you need any store at all.  The basic idea is that the log
is divided into individually verifiable segments.  For auditing
purposes you could keep all segments, so you have the entire log, but
if you've acted on the prior log entries and you don't have an audit
reason to keep them, you could erase that segment of the log because
you've placed all your trust in the prior log segments into the PCR
entry that forms the base of your current segment.

Essentially the question devolves to what mechanisms can give you this
trust in the base PCR log entry.

James



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 20:37       ` James Bottomley
@ 2021-01-07 20:51         ` Mimi Zohar
  2021-01-07 21:48           ` James Bottomley
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-01-07 20:51 UTC (permalink / raw)
  To: James Bottomley, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 12:37 -0800, James Bottomley wrote:
> On Thu, 2021-01-07 at 15:02 -0500, Mimi Zohar wrote:
> > On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
> > > On Thu, 2021-01-07 at 10:06 -0500, Mimi Zohar wrote:
> > > > [Cc: Amir Goldstein]
> > > > 
> > > > On Tue, 2021-01-05 at 11:57 -0800, Raphael Gianotti wrote:
> > > > > IMA measures files and buffer data and some systems may end up
> > > > > generating lots of entries in the IMA measurement list. This
> > > > > list is kept in kernel memoryc and as it grows in size it could
> > > > > end up taking too many resources, causing the system to run out
> > > > > of available memory. During kexec, the IMA measurement list can
> > > > > be carried over in memory, but it's possible for the list to
> > > > > become too large for that to happen.
> > > > > 
> > > > > The Kconfig introduced in this series enables admins to
> > > > > configure a maximum number of entries and a file to export the
> > > > > IMA measurement list to whenever the set limit is reached.
> > > > > 
> > > > > The list is written out in append mode, so the system will keep
> > > > > writing new entries as long as it stays running or runs out of
> > > > > space. Whenever the export file is set, it's truncated. If
> > > > > writing to the export list fails, a flag is set to prevent
> > > > > further exports, as the file is likely in a bad state. Setting
> > > > > a new export file resets this flag, allowing exports to resume
> > > > > and giving admins a way to recover from this state if
> > > > > necessary.
> > > > > 
> > > > > In the case of kexec, if the list is too large too be carried
> > > > > over in memory and an export file is configured, the list will
> > > > > be exported, preventing the measurements from being lost during
> > > > > kexec.
> > > > > 
> > > > > This code is based off of a previous RFC sent by Janne
> > > > > Karhunen[1], and is intended to pick up where that was left
> > > > > off.
> > > > > 
> > > > > In a thread with Janne Karhunen[2], it was mentioned that
> > > > > another approach, using mm had been considered. Upon some
> > > > > investigation the approach used in this RFC still seemed
> > > > > adequate for solving this problem.
> > > > > 
> > > > > [1] 
> > > > > https://patchwork.kernel.org/project/linux-integrity/patch/201912
> > > > > 20074929.8191-1-janne.karhunen@gmail.com/
> > > > > [2] 
> > > > > https://lore.kernel.org/linux-integrity/CAE=NcrbdS-3gVvnnEwdNSOLO
> > > > > vTenLjyppDz2aJACGRgBYSh=Gw@mail.gmail.com/
> > > > > 
> > > > > Signed-off-by: Raphael Gianotti <raphgi@linux.microsoft.com>
> > > > 
> > > > My original concerns of truncating the IMA measurement list have
> > > > not been addressed.  Once the IMA measurement list has been
> > > > truncated, quoting and then verifying any of the PCRs contained
> > > > in the measurement list will fail, unless the measurements have
> > > > been preserved and are readily accessible.
> > > > 
> > > > Amir's suggestion addresses kernel memory constraints without
> > > > truncating the IMA measurement list.
> > > 
> > > What about having a log entry that's the current PCR value?  Then
> > > stretches of the log starting with these entries would be
> > > independently verifiable provided you had a way of trusting the PCR
> > > value.  It might be possible to get the TPM to add a signed quote
> > > as an optional part of the log entry (of course this brings other
> > > problems like which key do you use for the signing and how does it
> > > get verified) which would provide the trust and would definitively
> > > allow you to archive log segments and still make the rest of the
> > > log useful.
> > 
> > The current PCR values are aggregated and stored in the
> > boot_aggregate record.  As part of the new boot_aggregate record
> > format, the individual PCR values could be included.
> 
> I don't think we care about the boot aggregate ... it's just the
> initial log entry that ties the boot state to the initial runtime
> state.  All we need for the proposed entry is the current value of the
> IMA PCR so provided you trust that value it becomes a base on which the
> following measurements can build and be trusted.

The IMA measurement list may contain multiple PCRs, not just the
default IMA PCR.   Each kexec results in an additional boot_aggregate
record, but an equivalent record for after truncating the measurement
list might help.
> 
> > But this doesn't address where the offloaded measurement list will be
> > stored, how long the list will be retained, nor who guarantees the
> > integrity of the offloaded list.  In addition, different form factors
> > will have different requirements.
> 
> I'm not sure you need any store at all.  The basic idea is that the log
> is divided into individually verifiable segments.  For auditing
> purposes you could keep all segments, so you have the entire log, but
> if you've acted on the prior log entries and you don't have an audit
> reason to keep them, you could erase that segment of the log because
> you've placed all your trust in the prior log segments into the PCR
> entry that forms the base of your current segment.
> 
> Essentially the question devolves to what mechanisms can give you this
> trust in the base PCR log entry.

Not retaining the entire measurement list would limit it's verification
to a single server/system.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 20:51         ` Mimi Zohar
@ 2021-01-07 21:48           ` James Bottomley
  2021-01-07 22:57             ` Raphael Gianotti
  2021-01-07 23:00             ` Mimi Zohar
  0 siblings, 2 replies; 23+ messages in thread
From: James Bottomley @ 2021-01-07 21:48 UTC (permalink / raw)
  To: Mimi Zohar, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 15:51 -0500, Mimi Zohar wrote:
> On Thu, 2021-01-07 at 12:37 -0800, James Bottomley wrote:
> > On Thu, 2021-01-07 at 15:02 -0500, Mimi Zohar wrote:
> > > On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
[...]
> > > > What about having a log entry that's the current PCR
> > > > value?  Then stretches of the log starting with these entries
> > > > would be independently verifiable provided you had a way of
> > > > trusting the PCR value.  It might be possible to get the TPM to
> > > > add a signed quote as an optional part of the log entry (of
> > > > course this brings other problems like which key do you use for
> > > > the signing and how does it get verified) which would provide
> > > > the trust and would definitively allow you to archive log
> > > > segments and still make the rest of the log useful.
> > > 
> > > The current PCR values are aggregated and stored in the
> > > boot_aggregate record.  As part of the new boot_aggregate record
> > > format, the individual PCR values could be included.
> > 
> > I don't think we care about the boot aggregate ... it's just the
> > initial log entry that ties the boot state to the initial runtime
> > state.  All we need for the proposed entry is the current value of
> > the IMA PCR so provided you trust that value it becomes a base on
> > which the following measurements can build and be trusted.
> 
> The IMA measurement list may contain multiple PCRs, not just the
> default IMA PCR.   Each kexec results in an additional boot_aggregate
> record, but an equivalent record for after truncating the measurement
> list might help.

Right, this would specifically be only of the IMA PCR so you can use it
as a base to begin the hash of the following log segment.  The log can
still contain other boot aggregate entries, but the assumption is that
boot aggregate entries in the prior log have already been evaluated.

> > > But this doesn't address where the offloaded measurement list
> > > will be stored, how long the list will be retained, nor who
> > > guarantees the integrity of the offloaded list.  In addition,
> > > different form factors will have different requirements.
> > 
> > I'm not sure you need any store at all.  The basic idea is that the
> > log is divided into individually verifiable segments.  For auditing
> > purposes you could keep all segments, so you have the entire log,
> > but if you've acted on the prior log entries and you don't have an
> > audit reason to keep them, you could erase that segment of the log
> > because you've placed all your trust in the prior log segments into
> > the PCR entry that forms the base of your current segment.
> > 
> > Essentially the question devolves to what mechanisms can give you
> > this trust in the base PCR log entry.
> 
> 
Not retaining the entire measurement list would limit it's verification
to a single server/system.

Well, it would limit its verification to just that log segment, yes.

I'm thinking in the cloud there are a couple of potential consumers:  

   1. The cloud monitor, which acts on the verified log, such as killing a
      node for trying to execute an unverified binary or emailing the
      guest owner.  This type of consumer doesn't need the historical log,
      they just need to verify the entries they haven't already seen and
      act on them according to whatever policy they're given.
   2. The second type of cloud consumer is the audit case where the
      aggregate hash is used to assure some auditor, some time after the
      actual events, that the entire runtime of the VM was properly
      monitored and the auditor wants to see the log  or a segment of it
      to prove the hash.

Case 1 doesn't need historical storage, case 2 definitely does.  I
think we should support both use cases particularly in the long running
scenario where we need to recover memory.  Having verifiable log
segments seems to satisfy both cases, but what you do with the segments
would vary.

James



^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 21:48           ` James Bottomley
@ 2021-01-07 22:57             ` Raphael Gianotti
  2021-01-08 12:38               ` Mimi Zohar
  2021-01-07 23:00             ` Mimi Zohar
  1 sibling, 1 reply; 23+ messages in thread
From: Raphael Gianotti @ 2021-01-07 22:57 UTC (permalink / raw)
  To: James Bottomley, Mimi Zohar, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein


On 1/7/2021 1:48 PM, James Bottomley wrote:
> On Thu, 2021-01-07 at 15:51 -0500, Mimi Zohar wrote:
>> On Thu, 2021-01-07 at 12:37 -0800, James Bottomley wrote:
>>> On Thu, 2021-01-07 at 15:02 -0500, Mimi Zohar wrote:
>>>> On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
> [...]
>>>>> What about having a log entry that's the current PCR
>>>>> value?  Then stretches of the log starting with these entries
>>>>> would be independently verifiable provided you had a way of
>>>>> trusting the PCR value.  It might be possible to get the TPM to
>>>>> add a signed quote as an optional part of the log entry (of
>>>>> course this brings other problems like which key do you use for
>>>>> the signing and how does it get verified) which would provide
>>>>> the trust and would definitively allow you to archive log
>>>>> segments and still make the rest of the log useful.
>>>> The current PCR values are aggregated and stored in the
>>>> boot_aggregate record.  As part of the new boot_aggregate record
>>>> format, the individual PCR values could be included.
>>> I don't think we care about the boot aggregate ... it's just the
>>> initial log entry that ties the boot state to the initial runtime
>>> state.  All we need for the proposed entry is the current value of
>>> the IMA PCR so provided you trust that value it becomes a base on
>>> which the following measurements can build and be trusted.
>> The IMA measurement list may contain multiple PCRs, not just the
>> default IMA PCR.   Each kexec results in an additional boot_aggregate
>> record, but an equivalent record for after truncating the measurement
>> list might help.
> Right, this would specifically be only of the IMA PCR so you can use it
> as a base to begin the hash of the following log segment.  The log can
> still contain other boot aggregate entries, but the assumption is that
> boot aggregate entries in the prior log have already been evaluated.
>
>>>> But this doesn't address where the offloaded measurement list
>>>> will be stored, how long the list will be retained, nor who
>>>> guarantees the integrity of the offloaded list.  In addition,
>>>> different form factors will have different requirements.

For how long the list would be retained, or in the case of a log segments, it
might make sense to have that be an admin decision, something that can be
configured to satisfy the needs of a specific system, as mentioned below by
James, does that seem correct?

Given the possibility of keeping the logs around for an indefinite amount of
time, would using an expansion of the method present in this RFC be more
appropriate than going down the vfs_tmpfile route? Forgive my lack on expertise
on mm, but would the vfs_tmpfile approach work for keeping several log segments
across multiple kexecs?

For how to guarantee the integrity of the offloaded logs, James suggestion
of using TPM for adding a signature to the log entries brings the question
of what key would be used and how it would be verified, I am trying to give
this some thought.

>>> I'm not sure you need any store at all.  The basic idea is that the
>>> log is divided into individually verifiable segments.  For auditing
>>> purposes you could keep all segments, so you have the entire log,
>>> but if you've acted on the prior log entries and you don't have an
>>> audit reason to keep them, you could erase that segment of the log
>>> because you've placed all your trust in the prior log segments into
>>> the PCR entry that forms the base of your current segment.
>>>
>>> Essentially the question devolves to what mechanisms can give you
>>> this trust in the base PCR log entry.
>>
> Not retaining the entire measurement list would limit it's verification
> to a single server/system.
>
> Well, it would limit its verification to just that log segment, yes.
>
> I'm thinking in the cloud there are a couple of potential consumers:
>
>     1. The cloud monitor, which acts on the verified log, such as killing a
>        node for trying to execute an unverified binary or emailing the
>        guest owner.  This type of consumer doesn't need the historical log,
>        they just need to verify the entries they haven't already seen and
>        act on them according to whatever policy they're given.
>     2. The second type of cloud consumer is the audit case where the
>        aggregate hash is used to assure some auditor, some time after the
>        actual events, that the entire runtime of the VM was properly
>        monitored and the auditor wants to see the log  or a segment of it
>        to prove the hash.
>
> Case 1 doesn't need historical storage, case 2 definitely does.  I
> think we should support both use cases particularly in the long running
> scenario where we need to recover memory.  Having verifiable log
> segments seems to satisfy both cases, but what you do with the segments
> would vary.
>
> James
>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 21:48           ` James Bottomley
  2021-01-07 22:57             ` Raphael Gianotti
@ 2021-01-07 23:00             ` Mimi Zohar
  1 sibling, 0 replies; 23+ messages in thread
From: Mimi Zohar @ 2021-01-07 23:00 UTC (permalink / raw)
  To: James Bottomley, Raphael Gianotti, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 13:48 -0800, James Bottomley wrote:
> On Thu, 2021-01-07 at 15:51 -0500, Mimi Zohar wrote:
> > On Thu, 2021-01-07 at 12:37 -0800, James Bottomley wrote:
> > > On Thu, 2021-01-07 at 15:02 -0500, Mimi Zohar wrote:
> > > > On Thu, 2021-01-07 at 08:42 -0800, James Bottomley wrote:
> [...]
> > > > > What about having a log entry that's the current PCR
> > > > > value?  Then stretches of the log starting with these entries
> > > > > would be independently verifiable provided you had a way of
> > > > > trusting the PCR value.  It might be possible to get the TPM to
> > > > > add a signed quote as an optional part of the log entry (of
> > > > > course this brings other problems like which key do you use for
> > > > > the signing and how does it get verified) which would provide
> > > > > the trust and would definitively allow you to archive log
> > > > > segments and still make the rest of the log useful.
> > > > 
> > > > The current PCR values are aggregated and stored in the
> > > > boot_aggregate record.  As part of the new boot_aggregate record
> > > > format, the individual PCR values could be included.
> > > 
> > > I don't think we care about the boot aggregate ... it's just the
> > > initial log entry that ties the boot state to the initial runtime
> > > state.  All we need for the proposed entry is the current value of
> > > the IMA PCR so provided you trust that value it becomes a base on
> > > which the following measurements can build and be trusted.
> > 
> > The IMA measurement list may contain multiple PCRs, not just the
> > default IMA PCR.   Each kexec results in an additional boot_aggregate
> > record, but an equivalent record for after truncating the measurement
> > list might help.
> 
> Right, this would specifically be only of the IMA PCR so you can use it
> as a base to begin the hash of the following log segment.  The log can
> still contain other boot aggregate entries, but the assumption is that
> boot aggregate entries in the prior log have already been evaluated.

IMA may be configured on a per rule basis to measure files into
different PCRs.  Between the previous boot aggregate record and
truncating the measurement list, other PCRs may have been extended.

> 
> > > > But this doesn't address where the offloaded measurement list
> > > > will be stored, how long the list will be retained, nor who
> > > > guarantees the integrity of the offloaded list.  In addition,
> > > > different form factors will have different requirements.
> > > 
> > > I'm not sure you need any store at all.  The basic idea is that the
> > > log is divided into individually verifiable segments.  For auditing
> > > purposes you could keep all segments, so you have the entire log,
> > > but if you've acted on the prior log entries and you don't have an
> > > audit reason to keep them, you could erase that segment of the log
> > > because you've placed all your trust in the prior log segments into
> > > the PCR entry that forms the base of your current segment.
> > > 
> > > Essentially the question devolves to what mechanisms can give you
> > > this trust in the base PCR log entry.
> > 
> > 
> > Not retaining the entire measurement list would limit it's verification
> > to a single server/system.
> 
> Well, it would limit its verification to just that log segment, yes.

Verifying only the current log segment only makes sense, if the
previous log segments were previously verified.

> 
> I'm thinking in the cloud there are a couple of potential consumers:  
> 
>    1. The cloud monitor, which acts on the verified log, such as killing a
>       node for trying to execute an unverified binary or emailing the
>       guest owner.  This type of consumer doesn't need the historical log,
>       they just need to verify the entries they haven't already seen and
>       act on them according to whatever policy they're given.
>    2. The second type of cloud consumer is the audit case where the
>       aggregate hash is used to assure some auditor, some time after the
>       actual events, that the entire runtime of the VM was properly
>       monitored and the auditor wants to see the log  or a segment of it
>       to prove the hash.
> 
> Case 1 doesn't need historical storage, case 2 definitely does.  I
> think we should support both use cases particularly in the long running
> scenario where we need to recover memory.  Having verifiable log
> segments seems to satisfy both cases, but what you do with the segments
> would vary.

As Ken previously pointed out, the attestation server itself can
request only the new measurements.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-07 22:57             ` Raphael Gianotti
@ 2021-01-08 12:38               ` Mimi Zohar
  2021-01-08 17:58                 ` Raphael Gianotti
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-01-08 12:38 UTC (permalink / raw)
  To: Raphael Gianotti, James Bottomley, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein

On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> >>>> But this doesn't address where the offloaded measurement list
> >>>> will be stored, how long the list will be retained, nor who
> >>>> guarantees the integrity of the offloaded list.  In addition,
> >>>> different form factors will have different requirements.
> 
> For how long the list would be retained, or in the case of a log segments, it
> might make sense to have that be an admin decision, something that can be
> configured to satisfy the needs of a specific system, as mentioned below by
> James, does that seem correct?

For the discussion on exporting and truncating the IMA measurement
list, refer to: 
https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/

> 
> Given the possibility of keeping the logs around for an indefinite amount of
> time, would using an expansion of the method present in this RFC be more
> appropriate than going down the vfs_tmpfile route? Forgive my lack on expertise
> on mm, but would the vfs_tmpfile approach work for keeping several log segments
> across multiple kexecs?

With the "vfs_tmpfile" mechanism, breaking up and saving the log in
segments isn't needed.  The existing mechanism for carrying the
measurement list across kexec would still be used.  Currently, if the
kernel cannot allocate the memory needed for carrying the measurement
across kexec, it simply emits an error message, but continues with the
kexec.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-08 12:38               ` Mimi Zohar
@ 2021-01-08 17:58                 ` Raphael Gianotti
  2021-02-01 22:53                   ` Raphael Gianotti
  0 siblings, 1 reply; 23+ messages in thread
From: Raphael Gianotti @ 2021-01-08 17:58 UTC (permalink / raw)
  To: Mimi Zohar, James Bottomley, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein


On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
>>>>>> But this doesn't address where the offloaded measurement list
>>>>>> will be stored, how long the list will be retained, nor who
>>>>>> guarantees the integrity of the offloaded list.  In addition,
>>>>>> different form factors will have different requirements.
>> For how long the list would be retained, or in the case of a log segments, it
>> might make sense to have that be an admin decision, something that can be
>> configured to satisfy the needs of a specific system, as mentioned below by
>> James, does that seem correct?
> For the discussion on exporting and truncating the IMA measurement
> list, refer to:
> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
>
>> Given the possibility of keeping the logs around for an indefinite amount of
>> time, would using an expansion of the method present in this RFC be more
>> appropriate than going down the vfs_tmpfile route? Forgive my lack on expertise
>> on mm, but would the vfs_tmpfile approach work for keeping several log segments
>> across multiple kexecs?
> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> segments isn't needed.  The existing mechanism for carrying the
> measurement list across kexec would still be used.  Currently, if the
> kernel cannot allocate the memory needed for carrying the measurement
> across kexec, it simply emits an error message, but continues with the
> kexec.

In this change I had introduced "exporting" the log to disk when the size
of the measurement list was too large. Given part of the motivation behind
moving the measurement list is the possibility of it growing too large
and taking up too much of the kernel memory, that case would likely lead
to kexec not being able to carry over the logs. Do you believe it's better
to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
about potential issues with kexec not being able to carry over the logs
separately, given the "vfs_tempfile" approach seems to be preferred and
also simplifies worries regarding truncating the logs?

>
> Mimi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-01-08 17:58                 ` Raphael Gianotti
@ 2021-02-01 22:53                   ` Raphael Gianotti
  2021-02-02  5:54                     ` Amir Goldstein
  0 siblings, 1 reply; 23+ messages in thread
From: Raphael Gianotti @ 2021-02-01 22:53 UTC (permalink / raw)
  To: Mimi Zohar, James Bottomley, janne.karhunen
  Cc: linux-integrity, tusharsu, tyhicks, nramas, balajib, Amir Goldstein


On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
>
> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
>>>>>>> But this doesn't address where the offloaded measurement list
>>>>>>> will be stored, how long the list will be retained, nor who
>>>>>>> guarantees the integrity of the offloaded list.  In addition,
>>>>>>> different form factors will have different requirements.
>>> For how long the list would be retained, or in the case of a log 
>>> segments, it
>>> might make sense to have that be an admin decision, something that 
>>> can be
>>> configured to satisfy the needs of a specific system, as mentioned 
>>> below by
>>> James, does that seem correct?
>> For the discussion on exporting and truncating the IMA measurement
>> list, refer to:
>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/ 
>>
>>
>>> Given the possibility of keeping the logs around for an indefinite 
>>> amount of
>>> time, would using an expansion of the method present in this RFC be 
>>> more
>>> appropriate than going down the vfs_tmpfile route? Forgive my lack 
>>> on expertise
>>> on mm, but would the vfs_tmpfile approach work for keeping several 
>>> log segments
>>> across multiple kexecs?
>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
>> segments isn't needed.  The existing mechanism for carrying the
>> measurement list across kexec would still be used.  Currently, if the
>> kernel cannot allocate the memory needed for carrying the measurement
>> across kexec, it simply emits an error message, but continues with the
>> kexec.
>
> In this change I had introduced "exporting" the log to disk when the size
> of the measurement list was too large. Given part of the motivation 
> behind
> moving the measurement list is the possibility of it growing too large
> and taking up too much of the kernel memory, that case would likely lead
> to kexec not being able to carry over the logs. Do you believe it's 
> better
> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> about potential issues with kexec not being able to carry over the logs
> separately, given the "vfs_tempfile" approach seems to be preferred and
> also simplifies worries regarding truncating the logs?

After a chat with Mimi I went ahead and did some investigative
work in the vfs_tmpfile approach suggested, and I wanted to
restart this thread with some thoughts/questions that came up
from that.
For the work I did I simply created a tmp file during ima's
initialization and then tried to use vm_mmap to map it to memory,
with the goal of using that memory mapping to generate return
pointers to the code that writes the measurement entries to memory.
What I found out is that vm_mmap creates a user space address
mapping, which can't be directly written to/read by kernel code,
am I missing something here ? I looked throughout the code and
read the implementation of vm_mmap and the functions it calls,
but I couldn't figure out if there's a version of this type of
mapping that returns a kernel space address.
Out of curiosity, I also tried (simply to test the approach),
translating the mapped address into a valid kernel address
by using find_vma(),  follow_page() and kmap(). I was able
to get a return value from kmap() that passed an IS_ERR() check,
however I couldn't get that to work properly when trying to
access it to read to/write from it. Again, I feel like the
mapping should be done directly to a valid kernel address,
but I wanted tried this to at least be able to prototype the
overall flow of the change.

Raph

>
>>
>> Mimi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-01 22:53                   ` Raphael Gianotti
@ 2021-02-02  5:54                     ` Amir Goldstein
  2021-02-02 13:07                       ` Mimi Zohar
  0 siblings, 1 reply; 23+ messages in thread
From: Amir Goldstein @ 2021-02-02  5:54 UTC (permalink / raw)
  To: Raphael Gianotti
  Cc: Mimi Zohar, James Bottomley, janne.karhunen, linux-integrity,
	tusharsu, tyhicks, nramas, balajib

On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
<raphgi@linux.microsoft.com> wrote:
>
>
> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> >
> > On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> >> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> >>>>>>> But this doesn't address where the offloaded measurement list
> >>>>>>> will be stored, how long the list will be retained, nor who
> >>>>>>> guarantees the integrity of the offloaded list.  In addition,
> >>>>>>> different form factors will have different requirements.
> >>> For how long the list would be retained, or in the case of a log
> >>> segments, it
> >>> might make sense to have that be an admin decision, something that
> >>> can be
> >>> configured to satisfy the needs of a specific system, as mentioned
> >>> below by
> >>> James, does that seem correct?
> >> For the discussion on exporting and truncating the IMA measurement
> >> list, refer to:
> >> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> >>
> >>
> >>> Given the possibility of keeping the logs around for an indefinite
> >>> amount of
> >>> time, would using an expansion of the method present in this RFC be
> >>> more
> >>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> >>> on expertise
> >>> on mm, but would the vfs_tmpfile approach work for keeping several
> >>> log segments
> >>> across multiple kexecs?
> >> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> >> segments isn't needed.  The existing mechanism for carrying the
> >> measurement list across kexec would still be used.  Currently, if the
> >> kernel cannot allocate the memory needed for carrying the measurement
> >> across kexec, it simply emits an error message, but continues with the
> >> kexec.
> >
> > In this change I had introduced "exporting" the log to disk when the size
> > of the measurement list was too large. Given part of the motivation
> > behind
> > moving the measurement list is the possibility of it growing too large
> > and taking up too much of the kernel memory, that case would likely lead
> > to kexec not being able to carry over the logs. Do you believe it's
> > better
> > to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> > about potential issues with kexec not being able to carry over the logs
> > separately, given the "vfs_tempfile" approach seems to be preferred and
> > also simplifies worries regarding truncating the logs?
>
> After a chat with Mimi I went ahead and did some investigative
> work in the vfs_tmpfile approach suggested, and I wanted to
> restart this thread with some thoughts/questions that came up
> from that.
> For the work I did I simply created a tmp file during ima's
> initialization and then tried to use vm_mmap to map it to memory,
> with the goal of using that memory mapping to generate return
> pointers to the code that writes the measurement entries to memory.

I don't understand why you would want to do that. I might have misunderstood
the requirements, but this was not how I meant for tmpfile to be used.

Mimi explained to me that currently the IMA measurement list is entirely in
memory and that you are looking for a way to dump it into a file in order to
free up memory.

What I suggested is this:

- User opens an O_TMPFILE and passes fd to IMA to start export
- IMA starts writing (exporting) records to that file using *kernel* write API
- Every record written to the file is removed from the in-memory list
- While list is being exported, IMA keeps in-memory count of exported entries
- In ima_measurements_start, if export file exists, start iterator
starts reading
  records from the file
- In ima_measurements_next(), when next iterator reaches the export count,
  it switches over to iterate in-memory list

This process can:
1. Continue forever without maintaining any in-memory list
2. Work in the background to periodically flush list to file
3. Controlled by explicit user commands
4. All of the above

Is that understood? Did I understand the requirements correctly?

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-02  5:54                     ` Amir Goldstein
@ 2021-02-02 13:07                       ` Mimi Zohar
  2021-02-02 18:14                         ` Raphael Gianotti
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-02-02 13:07 UTC (permalink / raw)
  To: Amir Goldstein, Raphael Gianotti
  Cc: James Bottomley, janne.karhunen, linux-integrity, tusharsu,
	tyhicks, nramas, balajib

On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
> <raphgi@linux.microsoft.com> wrote:
> >
> >
> > On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> > >
> > > On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> > >> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> > >>>>>>> But this doesn't address where the offloaded measurement list
> > >>>>>>> will be stored, how long the list will be retained, nor who
> > >>>>>>> guarantees the integrity of the offloaded list.  In addition,
> > >>>>>>> different form factors will have different requirements.
> > >>> For how long the list would be retained, or in the case of a log
> > >>> segments, it
> > >>> might make sense to have that be an admin decision, something that
> > >>> can be
> > >>> configured to satisfy the needs of a specific system, as mentioned
> > >>> below by
> > >>> James, does that seem correct?
> > >> For the discussion on exporting and truncating the IMA measurement
> > >> list, refer to:
> > >> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> > >>
> > >>
> > >>> Given the possibility of keeping the logs around for an indefinite
> > >>> amount of
> > >>> time, would using an expansion of the method present in this RFC be
> > >>> more
> > >>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> > >>> on expertise
> > >>> on mm, but would the vfs_tmpfile approach work for keeping several
> > >>> log segments
> > >>> across multiple kexecs?
> > >> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> > >> segments isn't needed.  The existing mechanism for carrying the
> > >> measurement list across kexec would still be used.  Currently, if the
> > >> kernel cannot allocate the memory needed for carrying the measurement
> > >> across kexec, it simply emits an error message, but continues with the
> > >> kexec.
> > >
> > > In this change I had introduced "exporting" the log to disk when the size
> > > of the measurement list was too large. Given part of the motivation
> > > behind
> > > moving the measurement list is the possibility of it growing too large
> > > and taking up too much of the kernel memory, that case would likely lead
> > > to kexec not being able to carry over the logs. Do you believe it's
> > > better
> > > to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> > > about potential issues with kexec not being able to carry over the logs
> > > separately, given the "vfs_tempfile" approach seems to be preferred and
> > > also simplifies worries regarding truncating the logs?
> >
> > After a chat with Mimi I went ahead and did some investigative
> > work in the vfs_tmpfile approach suggested, and I wanted to
> > restart this thread with some thoughts/questions that came up
> > from that.
> > For the work I did I simply created a tmp file during ima's
> > initialization and then tried to use vm_mmap to map it to memory,
> > with the goal of using that memory mapping to generate return
> > pointers to the code that writes the measurement entries to memory.
> 
> I don't understand why you would want to do that. I might have misunderstood
> the requirements, but this was not how I meant for tmpfile to be used.
> 
> Mimi explained to me that currently the IMA measurement list is entirely in
> memory and that you are looking for a way to dump it into a file in order to
> free up memory.
> 
> What I suggested is this:
> 
> - User opens an O_TMPFILE and passes fd to IMA to start export
> - IMA starts writing (exporting) records to that file using *kernel* write API
> - Every record written to the file is removed from the in-memory list
> - While list is being exported, IMA keeps in-memory count of exported entries
> - In ima_measurements_start, if export file exists, start iterator
> starts reading
>   records from the file
> - In ima_measurements_next(), when next iterator reaches the export count,
>   it switches over to iterate in-memory list
> 
> This process can:
> 1. Continue forever without maintaining any in-memory list
> 2. Work in the background to periodically flush list to file
> 3. Controlled by explicit user commands
> 4. All of the above
> 
> Is that understood? Did I understand the requirements correctly?

Thanks, Amir!

One of the main requirements we discussed was protecting the O_TMPFILE,
not allowing userspace direct access to the file.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-02 13:07                       ` Mimi Zohar
@ 2021-02-02 18:14                         ` Raphael Gianotti
  2021-02-03  1:02                           ` Mimi Zohar
  0 siblings, 1 reply; 23+ messages in thread
From: Raphael Gianotti @ 2021-02-02 18:14 UTC (permalink / raw)
  To: Mimi Zohar, Amir Goldstein
  Cc: James Bottomley, janne.karhunen, linux-integrity, tusharsu,
	tyhicks, nramas, balajib


On 2/2/2021 5:07 AM, Mimi Zohar wrote:
> On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
>> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
>> <raphgi@linux.microsoft.com> wrote:
>>>
>>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
>>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
>>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
>>>>>>>>>> But this doesn't address where the offloaded measurement list
>>>>>>>>>> will be stored, how long the list will be retained, nor who
>>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
>>>>>>>>>> different form factors will have different requirements.
>>>>>> For how long the list would be retained, or in the case of a log
>>>>>> segments, it
>>>>>> might make sense to have that be an admin decision, something that
>>>>>> can be
>>>>>> configured to satisfy the needs of a specific system, as mentioned
>>>>>> below by
>>>>>> James, does that seem correct?
>>>>> For the discussion on exporting and truncating the IMA measurement
>>>>> list, refer to:
>>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
>>>>>
>>>>>
>>>>>> Given the possibility of keeping the logs around for an indefinite
>>>>>> amount of
>>>>>> time, would using an expansion of the method present in this RFC be
>>>>>> more
>>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
>>>>>> on expertise
>>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
>>>>>> log segments
>>>>>> across multiple kexecs?
>>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
>>>>> segments isn't needed.  The existing mechanism for carrying the
>>>>> measurement list across kexec would still be used.  Currently, if the
>>>>> kernel cannot allocate the memory needed for carrying the measurement
>>>>> across kexec, it simply emits an error message, but continues with the
>>>>> kexec.
>>>> In this change I had introduced "exporting" the log to disk when the size
>>>> of the measurement list was too large. Given part of the motivation
>>>> behind
>>>> moving the measurement list is the possibility of it growing too large
>>>> and taking up too much of the kernel memory, that case would likely lead
>>>> to kexec not being able to carry over the logs. Do you believe it's
>>>> better
>>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
>>>> about potential issues with kexec not being able to carry over the logs
>>>> separately, given the "vfs_tempfile" approach seems to be preferred and
>>>> also simplifies worries regarding truncating the logs?
>>> After a chat with Mimi I went ahead and did some investigative
>>> work in the vfs_tmpfile approach suggested, and I wanted to
>>> restart this thread with some thoughts/questions that came up
>>> from that.
>>> For the work I did I simply created a tmp file during ima's
>>> initialization and then tried to use vm_mmap to map it to memory,
>>> with the goal of using that memory mapping to generate return
>>> pointers to the code that writes the measurement entries to memory.
>> I don't understand why you would want to do that. I might have misunderstood
>> the requirements, but this was not how I meant for tmpfile to be used.
>>
>> Mimi explained to me that currently the IMA measurement list is entirely in
>> memory and that you are looking for a way to dump it into a file in order to
>> free up memory.
>>
>> What I suggested is this:
>>
>> - User opens an O_TMPFILE and passes fd to IMA to start export
>> - IMA starts writing (exporting) records to that file using *kernel* write API
>> - Every record written to the file is removed from the in-memory list
>> - While list is being exported, IMA keeps in-memory count of exported entries
>> - In ima_measurements_start, if export file exists, start iterator
>> starts reading
>>    records from the file
>> - In ima_measurements_next(), when next iterator reaches the export count,
>>    it switches over to iterate in-memory list
>>
>> This process can:
>> 1. Continue forever without maintaining any in-memory list
>> 2. Work in the background to periodically flush list to file
>> 3. Controlled by explicit user commands
>> 4. All of the above
>>
>> Is that understood? Did I understand the requirements correctly?

Thanks for the clarification Amir, I never actually saw your initial mails,
I apologize for the confusion, the use of mmap was something the original
author of the export ima logs to disk mentioned had been suggested, which
is why I went down that route.
Given the actual suggestion you originally had given, I believe the coding
of it is somewhat to the code I sent in the RFC in terms of approach (if we
were to have it do periodic flushes, for example). With the addition of
reads to the log starting with the file as the oldest logs will be there.
I believe the only difference there is whether the list is kept in a tmp
file or not, so with the tmp file approach it would be just to keep the
list out of memory (either partially or permanently), where with a permanent
file, the list would still be available after a cold boot for instance.

> Thanks, Amir!
>
> One of the main requirements we discussed was protecting the O_TMPFILE,
> not allowing userspace direct access to the file.
>
> Mimi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-02 18:14                         ` Raphael Gianotti
@ 2021-02-03  1:02                           ` Mimi Zohar
  2021-02-03  7:24                             ` Amir Goldstein
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-02-03  1:02 UTC (permalink / raw)
  To: Raphael Gianotti, Amir Goldstein
  Cc: James Bottomley, janne.karhunen, linux-integrity, tusharsu,
	tyhicks, nramas, balajib

On Tue, 2021-02-02 at 10:14 -0800, Raphael Gianotti wrote:
> On 2/2/2021 5:07 AM, Mimi Zohar wrote:
> > On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
> >> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
> >> <raphgi@linux.microsoft.com> wrote:
> >>>
> >>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> >>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> >>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> >>>>>>>>>> But this doesn't address where the offloaded measurement list
> >>>>>>>>>> will be stored, how long the list will be retained, nor who
> >>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
> >>>>>>>>>> different form factors will have different requirements.
> >>>>>> For how long the list would be retained, or in the case of a log
> >>>>>> segments, it
> >>>>>> might make sense to have that be an admin decision, something that
> >>>>>> can be
> >>>>>> configured to satisfy the needs of a specific system, as mentioned
> >>>>>> below by
> >>>>>> James, does that seem correct?
> >>>>> For the discussion on exporting and truncating the IMA measurement
> >>>>> list, refer to:
> >>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> >>>>>
> >>>>>
> >>>>>> Given the possibility of keeping the logs around for an indefinite
> >>>>>> amount of
> >>>>>> time, would using an expansion of the method present in this RFC be
> >>>>>> more
> >>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> >>>>>> on expertise
> >>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
> >>>>>> log segments
> >>>>>> across multiple kexecs?
> >>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> >>>>> segments isn't needed.  The existing mechanism for carrying the
> >>>>> measurement list across kexec would still be used.  Currently, if the
> >>>>> kernel cannot allocate the memory needed for carrying the measurement
> >>>>> across kexec, it simply emits an error message, but continues with the
> >>>>> kexec.
> >>>> In this change I had introduced "exporting" the log to disk when the size
> >>>> of the measurement list was too large. Given part of the motivation
> >>>> behind
> >>>> moving the measurement list is the possibility of it growing too large
> >>>> and taking up too much of the kernel memory, that case would likely lead
> >>>> to kexec not being able to carry over the logs. Do you believe it's
> >>>> better
> >>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> >>>> about potential issues with kexec not being able to carry over the logs
> >>>> separately, given the "vfs_tempfile" approach seems to be preferred and
> >>>> also simplifies worries regarding truncating the logs?
> >>> After a chat with Mimi I went ahead and did some investigative
> >>> work in the vfs_tmpfile approach suggested, and I wanted to
> >>> restart this thread with some thoughts/questions that came up
> >>> from that.
> >>> For the work I did I simply created a tmp file during ima's
> >>> initialization and then tried to use vm_mmap to map it to memory,
> >>> with the goal of using that memory mapping to generate return
> >>> pointers to the code that writes the measurement entries to memory.
> >> I don't understand why you would want to do that. I might have misunderstood
> >> the requirements, but this was not how I meant for tmpfile to be used.
> >>
> >> Mimi explained to me that currently the IMA measurement list is entirely in
> >> memory and that you are looking for a way to dump it into a file in order to
> >> free up memory.
> >>
> >> What I suggested is this:
> >>
> >> - User opens an O_TMPFILE and passes fd to IMA to start export
> >> - IMA starts writing (exporting) records to that file using *kernel* write API
> >> - Every record written to the file is removed from the in-memory list
> >> - While list is being exported, IMA keeps in-memory count of exported entries
> >> - In ima_measurements_start, if export file exists, start iterator
> >> starts reading
> >>    records from the file
> >> - In ima_measurements_next(), when next iterator reaches the export count,
> >>    it switches over to iterate in-memory list
> >>
> >> This process can:
> >> 1. Continue forever without maintaining any in-memory list
> >> 2. Work in the background to periodically flush list to file
> >> 3. Controlled by explicit user commands
> >> 4. All of the above
> >>
> >> Is that understood? Did I understand the requirements correctly?
> 
> Thanks for the clarification Amir, I never actually saw your initial mails,
> I apologize for the confusion, the use of mmap was something the original
> author of the export ima logs to disk mentioned had been suggested, which
> is why I went down that route.
> Given the actual suggestion you originally had given, I believe the coding
> of it is somewhat to the code I sent in the RFC in terms of approach (if we
> were to have it do periodic flushes, for example). With the addition of
> reads to the log starting with the file as the oldest logs will be there.
> I believe the only difference there is whether the list is kept in a tmp
> file or not, so with the tmp file approach it would be just to keep the
> list out of memory (either partially or permanently), where with a permanent
> file, the list would still be available after a cold boot for instance.

With Amir's suggestion, userspace still accesses the entire measurement
list via the existing securityfs interface.  Only the kernel should be
able to append or access the file.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-03  1:02                           ` Mimi Zohar
@ 2021-02-03  7:24                             ` Amir Goldstein
  2021-02-03 18:45                               ` Mimi Zohar
  0 siblings, 1 reply; 23+ messages in thread
From: Amir Goldstein @ 2021-02-03  7:24 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Raphael Gianotti, James Bottomley, janne.karhunen,
	linux-integrity, tusharsu, tyhicks, nramas, balajib

On Wed, Feb 3, 2021 at 3:02 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>
> On Tue, 2021-02-02 at 10:14 -0800, Raphael Gianotti wrote:
> > On 2/2/2021 5:07 AM, Mimi Zohar wrote:
> > > On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
> > >> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
> > >> <raphgi@linux.microsoft.com> wrote:
> > >>>
> > >>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> > >>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> > >>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> > >>>>>>>>>> But this doesn't address where the offloaded measurement list
> > >>>>>>>>>> will be stored, how long the list will be retained, nor who
> > >>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
> > >>>>>>>>>> different form factors will have different requirements.
> > >>>>>> For how long the list would be retained, or in the case of a log
> > >>>>>> segments, it
> > >>>>>> might make sense to have that be an admin decision, something that
> > >>>>>> can be
> > >>>>>> configured to satisfy the needs of a specific system, as mentioned
> > >>>>>> below by
> > >>>>>> James, does that seem correct?
> > >>>>> For the discussion on exporting and truncating the IMA measurement
> > >>>>> list, refer to:
> > >>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> > >>>>>
> > >>>>>
> > >>>>>> Given the possibility of keeping the logs around for an indefinite
> > >>>>>> amount of
> > >>>>>> time, would using an expansion of the method present in this RFC be
> > >>>>>> more
> > >>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> > >>>>>> on expertise
> > >>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
> > >>>>>> log segments
> > >>>>>> across multiple kexecs?
> > >>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> > >>>>> segments isn't needed.  The existing mechanism for carrying the
> > >>>>> measurement list across kexec would still be used.  Currently, if the
> > >>>>> kernel cannot allocate the memory needed for carrying the measurement
> > >>>>> across kexec, it simply emits an error message, but continues with the
> > >>>>> kexec.
> > >>>> In this change I had introduced "exporting" the log to disk when the size
> > >>>> of the measurement list was too large. Given part of the motivation
> > >>>> behind
> > >>>> moving the measurement list is the possibility of it growing too large
> > >>>> and taking up too much of the kernel memory, that case would likely lead
> > >>>> to kexec not being able to carry over the logs. Do you believe it's
> > >>>> better
> > >>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> > >>>> about potential issues with kexec not being able to carry over the logs
> > >>>> separately, given the "vfs_tempfile" approach seems to be preferred and
> > >>>> also simplifies worries regarding truncating the logs?
> > >>> After a chat with Mimi I went ahead and did some investigative
> > >>> work in the vfs_tmpfile approach suggested, and I wanted to
> > >>> restart this thread with some thoughts/questions that came up
> > >>> from that.
> > >>> For the work I did I simply created a tmp file during ima's
> > >>> initialization and then tried to use vm_mmap to map it to memory,
> > >>> with the goal of using that memory mapping to generate return
> > >>> pointers to the code that writes the measurement entries to memory.
> > >> I don't understand why you would want to do that. I might have misunderstood
> > >> the requirements, but this was not how I meant for tmpfile to be used.
> > >>
> > >> Mimi explained to me that currently the IMA measurement list is entirely in
> > >> memory and that you are looking for a way to dump it into a file in order to
> > >> free up memory.
> > >>
> > >> What I suggested is this:
> > >>
> > >> - User opens an O_TMPFILE and passes fd to IMA to start export
> > >> - IMA starts writing (exporting) records to that file using *kernel* write API
> > >> - Every record written to the file is removed from the in-memory list
> > >> - While list is being exported, IMA keeps in-memory count of exported entries
> > >> - In ima_measurements_start, if export file exists, start iterator
> > >> starts reading
> > >>    records from the file
> > >> - In ima_measurements_next(), when next iterator reaches the export count,
> > >>    it switches over to iterate in-memory list
> > >>
> > >> This process can:
> > >> 1. Continue forever without maintaining any in-memory list
> > >> 2. Work in the background to periodically flush list to file
> > >> 3. Controlled by explicit user commands
> > >> 4. All of the above
> > >>
> > >> Is that understood? Did I understand the requirements correctly?
> >
> > Thanks for the clarification Amir, I never actually saw your initial mails,
> > I apologize for the confusion, the use of mmap was something the original
> > author of the export ima logs to disk mentioned had been suggested, which
> > is why I went down that route.
> > Given the actual suggestion you originally had given, I believe the coding
> > of it is somewhat to the code I sent in the RFC in terms of approach (if we
> > were to have it do periodic flushes, for example). With the addition of
> > reads to the log starting with the file as the oldest logs will be there.
> > I believe the only difference there is whether the list is kept in a tmp
> > file or not, so with the tmp file approach it would be just to keep the
> > list out of memory (either partially or permanently), where with a permanent
> > file, the list would still be available after a cold boot for instance.
>
> With Amir's suggestion, userspace still accesses the entire measurement
> list via the existing securityfs interface.  Only the kernel should be
> able to append or access the file.
>

This user API is not an important part of the suggestion:

- User opens an O_TMPFILE and passes fd to IMA to start export

It is just how I understood the API should be.
Kernel could open the O_TMPFILE or named file for that matter just as well.
If the kernel opens an O_TMPFILE, userspace has no standard way to access
that file. There are, as always, ways for privileged users to learn about that
tmpfile and open it with open_by_handle_at().

IMA is an LSM, so the best way to block unauthorized access to that file
would be via LSM hooks. IMA keeps a reference to that file, so it can
identify access to that file from userspace.

Thanks,
Amir.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-03  7:24                             ` Amir Goldstein
@ 2021-02-03 18:45                               ` Mimi Zohar
  2021-02-09 17:20                                 ` Raphael Gianotti
  0 siblings, 1 reply; 23+ messages in thread
From: Mimi Zohar @ 2021-02-03 18:45 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Raphael Gianotti, James Bottomley, janne.karhunen,
	linux-integrity, tusharsu, tyhicks, nramas, balajib

On Wed, 2021-02-03 at 09:24 +0200, Amir Goldstein wrote:
> On Wed, Feb 3, 2021 at 3:02 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >
> > On Tue, 2021-02-02 at 10:14 -0800, Raphael Gianotti wrote:
> > > On 2/2/2021 5:07 AM, Mimi Zohar wrote:
> > > > On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
> > > >> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
> > > >> <raphgi@linux.microsoft.com> wrote:
> > > >>>
> > > >>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> > > >>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> > > >>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> > > >>>>>>>>>> But this doesn't address where the offloaded measurement list
> > > >>>>>>>>>> will be stored, how long the list will be retained, nor who
> > > >>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
> > > >>>>>>>>>> different form factors will have different requirements.
> > > >>>>>> For how long the list would be retained, or in the case of a log
> > > >>>>>> segments, it
> > > >>>>>> might make sense to have that be an admin decision, something that
> > > >>>>>> can be
> > > >>>>>> configured to satisfy the needs of a specific system, as mentioned
> > > >>>>>> below by
> > > >>>>>> James, does that seem correct?
> > > >>>>> For the discussion on exporting and truncating the IMA measurement
> > > >>>>> list, refer to:
> > > >>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> > > >>>>>
> > > >>>>>
> > > >>>>>> Given the possibility of keeping the logs around for an indefinite
> > > >>>>>> amount of
> > > >>>>>> time, would using an expansion of the method present in this RFC be
> > > >>>>>> more
> > > >>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> > > >>>>>> on expertise
> > > >>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
> > > >>>>>> log segments
> > > >>>>>> across multiple kexecs?
> > > >>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> > > >>>>> segments isn't needed.  The existing mechanism for carrying the
> > > >>>>> measurement list across kexec would still be used.  Currently, if the
> > > >>>>> kernel cannot allocate the memory needed for carrying the measurement
> > > >>>>> across kexec, it simply emits an error message, but continues with the
> > > >>>>> kexec.
> > > >>>> In this change I had introduced "exporting" the log to disk when the size
> > > >>>> of the measurement list was too large. Given part of the motivation
> > > >>>> behind
> > > >>>> moving the measurement list is the possibility of it growing too large
> > > >>>> and taking up too much of the kernel memory, that case would likely lead
> > > >>>> to kexec not being able to carry over the logs. Do you believe it's
> > > >>>> better
> > > >>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> > > >>>> about potential issues with kexec not being able to carry over the logs
> > > >>>> separately, given the "vfs_tempfile" approach seems to be preferred and
> > > >>>> also simplifies worries regarding truncating the logs?
> > > >>> After a chat with Mimi I went ahead and did some investigative
> > > >>> work in the vfs_tmpfile approach suggested, and I wanted to
> > > >>> restart this thread with some thoughts/questions that came up
> > > >>> from that.
> > > >>> For the work I did I simply created a tmp file during ima's
> > > >>> initialization and then tried to use vm_mmap to map it to memory,
> > > >>> with the goal of using that memory mapping to generate return
> > > >>> pointers to the code that writes the measurement entries to memory.
> > > >> I don't understand why you would want to do that. I might have misunderstood
> > > >> the requirements, but this was not how I meant for tmpfile to be used.
> > > >>
> > > >> Mimi explained to me that currently the IMA measurement list is entirely in
> > > >> memory and that you are looking for a way to dump it into a file in order to
> > > >> free up memory.
> > > >>
> > > >> What I suggested is this:
> > > >>
> > > >> - User opens an O_TMPFILE and passes fd to IMA to start export
> > > >> - IMA starts writing (exporting) records to that file using *kernel* write API
> > > >> - Every record written to the file is removed from the in-memory list
> > > >> - While list is being exported, IMA keeps in-memory count of exported entries
> > > >> - In ima_measurements_start, if export file exists, start iterator
> > > >> starts reading
> > > >>    records from the file
> > > >> - In ima_measurements_next(), when next iterator reaches the export count,
> > > >>    it switches over to iterate in-memory list
> > > >>
> > > >> This process can:
> > > >> 1. Continue forever without maintaining any in-memory list
> > > >> 2. Work in the background to periodically flush list to file
> > > >> 3. Controlled by explicit user commands
> > > >> 4. All of the above
> > > >>
> > > >> Is that understood? Did I understand the requirements correctly?
> > >
> > > Thanks for the clarification Amir, I never actually saw your initial mails,
> > > I apologize for the confusion, the use of mmap was something the original
> > > author of the export ima logs to disk mentioned had been suggested, which
> > > is why I went down that route.
> > > Given the actual suggestion you originally had given, I believe the coding
> > > of it is somewhat to the code I sent in the RFC in terms of approach (if we
> > > were to have it do periodic flushes, for example). With the addition of
> > > reads to the log starting with the file as the oldest logs will be there.
> > > I believe the only difference there is whether the list is kept in a tmp
> > > file or not, so with the tmp file approach it would be just to keep the
> > > list out of memory (either partially or permanently), where with a permanent
> > > file, the list would still be available after a cold boot for instance.
> >
> > With Amir's suggestion, userspace still accesses the entire measurement
> > list via the existing securityfs interface.  Only the kernel should be
> > able to append or access the file.
> >
> 
> This user API is not an important part of the suggestion:
> 
> - User opens an O_TMPFILE and passes fd to IMA to start export
> 
> It is just how I understood the API should be.
> Kernel could open the O_TMPFILE or named file for that matter just as well.
> If the kernel opens an O_TMPFILE, userspace has no standard way to access
> that file. There are, as always, ways for privileged users to learn about that
> tmpfile and open it with open_by_handle_at().
> 
> IMA is an LSM, so the best way to block unauthorized access to that file
> would be via LSM hooks. IMA keeps a reference to that file, so it can
> identify access to that file from userspace.

Having the kernel open a O_TMPFILE and use/define additional LSM hooks,
as needed, to limit access to the file sounds good.

In terms of the rest of the userspace interface, I would probably
define a new IMA securityfs file to control the frequency that the
measurements are written to the file (e.g. 0 == never, 1 == enabled
with default frequency, anything else frequency).

thanks,

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-03 18:45                               ` Mimi Zohar
@ 2021-02-09 17:20                                 ` Raphael Gianotti
  2021-02-09 18:08                                   ` Mimi Zohar
  0 siblings, 1 reply; 23+ messages in thread
From: Raphael Gianotti @ 2021-02-09 17:20 UTC (permalink / raw)
  To: Mimi Zohar, Amir Goldstein
  Cc: James Bottomley, janne.karhunen, linux-integrity, tusharsu,
	tyhicks, nramas, balajib

On 2/3/2021 10:45 AM, Mimi Zohar wrote:
> On Wed, 2021-02-03 at 09:24 +0200, Amir Goldstein wrote:
>> On Wed, Feb 3, 2021 at 3:02 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> On Tue, 2021-02-02 at 10:14 -0800, Raphael Gianotti wrote:
>>>> On 2/2/2021 5:07 AM, Mimi Zohar wrote:
>>>>> On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
>>>>>> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
>>>>>> <raphgi@linux.microsoft.com> wrote:
>>>>>>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
>>>>>>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
>>>>>>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
>>>>>>>>>>>>>> But this doesn't address where the offloaded measurement list
>>>>>>>>>>>>>> will be stored, how long the list will be retained, nor who
>>>>>>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
>>>>>>>>>>>>>> different form factors will have different requirements.
>>>>>>>>>> For how long the list would be retained, or in the case of a log
>>>>>>>>>> segments, it
>>>>>>>>>> might make sense to have that be an admin decision, something that
>>>>>>>>>> can be
>>>>>>>>>> configured to satisfy the needs of a specific system, as mentioned
>>>>>>>>>> below by
>>>>>>>>>> James, does that seem correct?
>>>>>>>>> For the discussion on exporting and truncating the IMA measurement
>>>>>>>>> list, refer to:
>>>>>>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>> Given the possibility of keeping the logs around for an indefinite
>>>>>>>>>> amount of
>>>>>>>>>> time, would using an expansion of the method present in this RFC be
>>>>>>>>>> more
>>>>>>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
>>>>>>>>>> on expertise
>>>>>>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
>>>>>>>>>> log segments
>>>>>>>>>> across multiple kexecs?
>>>>>>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
>>>>>>>>> segments isn't needed.  The existing mechanism for carrying the
>>>>>>>>> measurement list across kexec would still be used.  Currently, if the
>>>>>>>>> kernel cannot allocate the memory needed for carrying the measurement
>>>>>>>>> across kexec, it simply emits an error message, but continues with the
>>>>>>>>> kexec.
>>>>>>>> In this change I had introduced "exporting" the log to disk when the size
>>>>>>>> of the measurement list was too large. Given part of the motivation
>>>>>>>> behind
>>>>>>>> moving the measurement list is the possibility of it growing too large
>>>>>>>> and taking up too much of the kernel memory, that case would likely lead
>>>>>>>> to kexec not being able to carry over the logs. Do you believe it's
>>>>>>>> better
>>>>>>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
>>>>>>>> about potential issues with kexec not being able to carry over the logs
>>>>>>>> separately, given the "vfs_tempfile" approach seems to be preferred and
>>>>>>>> also simplifies worries regarding truncating the logs?
>>>>>>> After a chat with Mimi I went ahead and did some investigative
>>>>>>> work in the vfs_tmpfile approach suggested, and I wanted to
>>>>>>> restart this thread with some thoughts/questions that came up
>>>>>>> from that.
>>>>>>> For the work I did I simply created a tmp file during ima's
>>>>>>> initialization and then tried to use vm_mmap to map it to memory,
>>>>>>> with the goal of using that memory mapping to generate return
>>>>>>> pointers to the code that writes the measurement entries to memory.
>>>>>> I don't understand why you would want to do that. I might have misunderstood
>>>>>> the requirements, but this was not how I meant for tmpfile to be used.
>>>>>>
>>>>>> Mimi explained to me that currently the IMA measurement list is entirely in
>>>>>> memory and that you are looking for a way to dump it into a file in order to
>>>>>> free up memory.
>>>>>>
>>>>>> What I suggested is this:
>>>>>>
>>>>>> - User opens an O_TMPFILE and passes fd to IMA to start export
>>>>>> - IMA starts writing (exporting) records to that file using *kernel* write API
>>>>>> - Every record written to the file is removed from the in-memory list
>>>>>> - While list is being exported, IMA keeps in-memory count of exported entries
>>>>>> - In ima_measurements_start, if export file exists, start iterator
>>>>>> starts reading
>>>>>>     records from the file
>>>>>> - In ima_measurements_next(), when next iterator reaches the export count,
>>>>>>     it switches over to iterate in-memory list
>>>>>>
>>>>>> This process can:
>>>>>> 1. Continue forever without maintaining any in-memory list
>>>>>> 2. Work in the background to periodically flush list to file
>>>>>> 3. Controlled by explicit user commands
>>>>>> 4. All of the above
>>>>>>
>>>>>> Is that understood? Did I understand the requirements correctly?
>>>> Thanks for the clarification Amir, I never actually saw your initial mails,
>>>> I apologize for the confusion, the use of mmap was something the original
>>>> author of the export ima logs to disk mentioned had been suggested, which
>>>> is why I went down that route.
>>>> Given the actual suggestion you originally had given, I believe the coding
>>>> of it is somewhat to the code I sent in the RFC in terms of approach (if we
>>>> were to have it do periodic flushes, for example). With the addition of
>>>> reads to the log starting with the file as the oldest logs will be there.
>>>> I believe the only difference there is whether the list is kept in a tmp
>>>> file or not, so with the tmp file approach it would be just to keep the
>>>> list out of memory (either partially or permanently), where with a permanent
>>>> file, the list would still be available after a cold boot for instance.
>>> With Amir's suggestion, userspace still accesses the entire measurement
>>> list via the existing securityfs interface.  Only the kernel should be
>>> able to append or access the file.
>>>
>> This user API is not an important part of the suggestion:
>>
>> - User opens an O_TMPFILE and passes fd to IMA to start export
>>
>> It is just how I understood the API should be.
>> Kernel could open the O_TMPFILE or named file for that matter just as well.
>> If the kernel opens an O_TMPFILE, userspace has no standard way to access
>> that file. There are, as always, ways for privileged users to learn about that
>> tmpfile and open it with open_by_handle_at().
>>
>> IMA is an LSM, so the best way to block unauthorized access to that file
>> would be via LSM hooks. IMA keeps a reference to that file, so it can
>> identify access to that file from userspace.
> Having the kernel open a O_TMPFILE and use/define additional LSM hooks,
> as needed, to limit access to the file sounds good.
>
> In terms of the rest of the userspace interface, I would probably
> define a new IMA securityfs file to control the frequency that the
> measurements are written to the file (e.g. 0 == never, 1 == enabled
> with default frequency, anything else frequency).
>
> thanks,
>
> Mimi

Regarding the comparison between the original approach in the RFC,
using permanent files, and the TMP file approach (with me having the
correct suggestion in mind now), I believe there are still some
benefits from having a file be permanent:
	- It provides a way to always keep the logs across the kexec
	  boundary, regardless of how much they've grown in size,
	  as the file will persist through it, where as with a tmp
	  file, the ability to persist the logs through kexec would
	  still be subject to the same memory restrictions it currently
	  is.
	- It provides a way for an attestation service to have a
	  history of measurements performed during previous cold boot
	  cycles.

The above, however, does introduce the requirements of persisting
PCR information that would allow verifying historical logs, and
that information would have to be verifiable in some way (for instance,
by having the TPM add a signature alongside the logs).

I think both solutions have their merits, and the TMP file approach
seems much simpler overall. What I have in mind is perhaps having this
be configurable, where a file can be defined to hold the logs, but
the persisting of logs to disk can still be turned on without that
file being configures, leading to a TMP file being used. That would
leave it up to the admin to decide whether a permanent file is needed.

The above is in addition to any other configurations that may be
applicable, such as one for the frequency as suggested by Mimi.

Thanks,
Raph


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFC] Persist ima logs to disk
  2021-02-09 17:20                                 ` Raphael Gianotti
@ 2021-02-09 18:08                                   ` Mimi Zohar
  0 siblings, 0 replies; 23+ messages in thread
From: Mimi Zohar @ 2021-02-09 18:08 UTC (permalink / raw)
  To: Raphael Gianotti, Amir Goldstein
  Cc: James Bottomley, janne.karhunen, linux-integrity, tusharsu,
	tyhicks, nramas, balajib

On Tue, 2021-02-09 at 09:20 -0800, Raphael Gianotti wrote:
> On 2/3/2021 10:45 AM, Mimi Zohar wrote:
> > On Wed, 2021-02-03 at 09:24 +0200, Amir Goldstein wrote:
> >> On Wed, Feb 3, 2021 at 3:02 AM Mimi Zohar <zohar@linux.ibm.com> wrote:
> >>> On Tue, 2021-02-02 at 10:14 -0800, Raphael Gianotti wrote:
> >>>> On 2/2/2021 5:07 AM, Mimi Zohar wrote:
> >>>>> On Tue, 2021-02-02 at 07:54 +0200, Amir Goldstein wrote:
> >>>>>> On Tue, Feb 2, 2021 at 12:53 AM Raphael Gianotti
> >>>>>> <raphgi@linux.microsoft.com> wrote:
> >>>>>>> On 1/8/2021 9:58 AM, Raphael Gianotti wrote:
> >>>>>>>> On 1/8/2021 4:38 AM, Mimi Zohar wrote:
> >>>>>>>>> On Thu, 2021-01-07 at 14:57 -0800, Raphael Gianotti wrote:
> >>>>>>>>>>>>>> But this doesn't address where the offloaded measurement list
> >>>>>>>>>>>>>> will be stored, how long the list will be retained, nor who
> >>>>>>>>>>>>>> guarantees the integrity of the offloaded list.  In addition,
> >>>>>>>>>>>>>> different form factors will have different requirements.
> >>>>>>>>>> For how long the list would be retained, or in the case of a log
> >>>>>>>>>> segments, it
> >>>>>>>>>> might make sense to have that be an admin decision, something that
> >>>>>>>>>> can be
> >>>>>>>>>> configured to satisfy the needs of a specific system, as mentioned
> >>>>>>>>>> below by
> >>>>>>>>>> James, does that seem correct?
> >>>>>>>>> For the discussion on exporting and truncating the IMA measurement
> >>>>>>>>> list, refer to:
> >>>>>>>>> https://lore.kernel.org/linux-integrity/1580998432.5585.411.camel@linux.ibm.com/
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> Given the possibility of keeping the logs around for an indefinite
> >>>>>>>>>> amount of
> >>>>>>>>>> time, would using an expansion of the method present in this RFC be
> >>>>>>>>>> more
> >>>>>>>>>> appropriate than going down the vfs_tmpfile route? Forgive my lack
> >>>>>>>>>> on expertise
> >>>>>>>>>> on mm, but would the vfs_tmpfile approach work for keeping several
> >>>>>>>>>> log segments
> >>>>>>>>>> across multiple kexecs?
> >>>>>>>>> With the "vfs_tmpfile" mechanism, breaking up and saving the log in
> >>>>>>>>> segments isn't needed.  The existing mechanism for carrying the
> >>>>>>>>> measurement list across kexec would still be used.  Currently, if the
> >>>>>>>>> kernel cannot allocate the memory needed for carrying the measurement
> >>>>>>>>> across kexec, it simply emits an error message, but continues with the
> >>>>>>>>> kexec.
> >>>>>>>> In this change I had introduced "exporting" the log to disk when the size
> >>>>>>>> of the measurement list was too large. Given part of the motivation
> >>>>>>>> behind
> >>>>>>>> moving the measurement list is the possibility of it growing too large
> >>>>>>>> and taking up too much of the kernel memory, that case would likely lead
> >>>>>>>> to kexec not being able to carry over the logs. Do you believe it's
> >>>>>>>> better
> >>>>>>>> to use the "vfs_tmpfile" mechanism for moving the logs to disk and worry
> >>>>>>>> about potential issues with kexec not being able to carry over the logs
> >>>>>>>> separately, given the "vfs_tempfile" approach seems to be preferred and
> >>>>>>>> also simplifies worries regarding truncating the logs?
> >>>>>>> After a chat with Mimi I went ahead and did some investigative
> >>>>>>> work in the vfs_tmpfile approach suggested, and I wanted to
> >>>>>>> restart this thread with some thoughts/questions that came up
> >>>>>>> from that.
> >>>>>>> For the work I did I simply created a tmp file during ima's
> >>>>>>> initialization and then tried to use vm_mmap to map it to memory,
> >>>>>>> with the goal of using that memory mapping to generate return
> >>>>>>> pointers to the code that writes the measurement entries to memory.
> >>>>>> I don't understand why you would want to do that. I might have misunderstood
> >>>>>> the requirements, but this was not how I meant for tmpfile to be used.
> >>>>>>
> >>>>>> Mimi explained to me that currently the IMA measurement list is entirely in
> >>>>>> memory and that you are looking for a way to dump it into a file in order to
> >>>>>> free up memory.
> >>>>>>
> >>>>>> What I suggested is this:
> >>>>>>
> >>>>>> - User opens an O_TMPFILE and passes fd to IMA to start export
> >>>>>> - IMA starts writing (exporting) records to that file using *kernel* write API
> >>>>>> - Every record written to the file is removed from the in-memory list
> >>>>>> - While list is being exported, IMA keeps in-memory count of exported entries
> >>>>>> - In ima_measurements_start, if export file exists, start iterator
> >>>>>> starts reading
> >>>>>>     records from the file
> >>>>>> - In ima_measurements_next(), when next iterator reaches the export count,
> >>>>>>     it switches over to iterate in-memory list
> >>>>>>
> >>>>>> This process can:
> >>>>>> 1. Continue forever without maintaining any in-memory list
> >>>>>> 2. Work in the background to periodically flush list to file
> >>>>>> 3. Controlled by explicit user commands
> >>>>>> 4. All of the above
> >>>>>>
> >>>>>> Is that understood? Did I understand the requirements correctly?
> >>>> Thanks for the clarification Amir, I never actually saw your initial mails,
> >>>> I apologize for the confusion, the use of mmap was something the original
> >>>> author of the export ima logs to disk mentioned had been suggested, which
> >>>> is why I went down that route.
> >>>> Given the actual suggestion you originally had given, I believe the coding
> >>>> of it is somewhat to the code I sent in the RFC in terms of approach (if we
> >>>> were to have it do periodic flushes, for example). With the addition of
> >>>> reads to the log starting with the file as the oldest logs will be there.
> >>>> I believe the only difference there is whether the list is kept in a tmp
> >>>> file or not, so with the tmp file approach it would be just to keep the
> >>>> list out of memory (either partially or permanently), where with a permanent
> >>>> file, the list would still be available after a cold boot for instance.
> >>> With Amir's suggestion, userspace still accesses the entire measurement
> >>> list via the existing securityfs interface.  Only the kernel should be
> >>> able to append or access the file.
> >>>
> >> This user API is not an important part of the suggestion:
> >>
> >> - User opens an O_TMPFILE and passes fd to IMA to start export
> >>
> >> It is just how I understood the API should be.
> >> Kernel could open the O_TMPFILE or named file for that matter just as well.
> >> If the kernel opens an O_TMPFILE, userspace has no standard way to access
> >> that file. There are, as always, ways for privileged users to learn about that
> >> tmpfile and open it with open_by_handle_at().
> >>
> >> IMA is an LSM, so the best way to block unauthorized access to that file
> >> would be via LSM hooks. IMA keeps a reference to that file, so it can
> >> identify access to that file from userspace.
> > Having the kernel open a O_TMPFILE and use/define additional LSM hooks,
> > as needed, to limit access to the file sounds good.
> >
> > In terms of the rest of the userspace interface, I would probably
> > define a new IMA securityfs file to control the frequency that the
> > measurements are written to the file (e.g. 0 == never, 1 == enabled
> > with default frequency, anything else frequency).
> >
> > thanks,
> >
> > Mimi
> 
> Regarding the comparison between the original approach in the RFC,
> using permanent files, and the TMP file approach (with me having the
> correct suggestion in mind now), I believe there are still some
> benefits from having a file be permanent:
> 	- It provides a way to always keep the logs across the kexec
> 	  boundary, regardless of how much they've grown in size,
> 	  as the file will persist through it, where as with a tmp
> 	  file, the ability to persist the logs through kexec would
> 	  still be subject to the same memory restrictions it currently
> 	  is.
> 	- It provides a way for an attestation service to have a
> 	  history of measurements performed during previous cold boot
> 	  cycles.
> 
> The above, however, does introduce the requirements of persisting
> PCR information that would allow verifying historical logs, and
> that information would have to be verifiable in some way (for instance,
> by having the TPM add a signature alongside the logs).
> 
> I think both solutions have their merits, and the TMP file approach
> seems much simpler overall. What I have in mind is perhaps having this
> be configurable, where a file can be defined to hold the logs, but
> the persisting of logs to disk can still be turned on without that
> file being configures, leading to a TMP file being used. That would
> leave it up to the admin to decide whether a permanent file is needed.
> 
> The above is in addition to any other configurations that may be
> applicable, such as one for the frequency as suggested by Mimi.

With the existing mechanism of exporting the IMA measurement list,
userspace can do whatever it likes with it and save it wherever it
likes (e.g. define a cron job to export the measurement list, the
number of measurements, and the TPM PCRs, even storing in a block
chain).  I don't see the benefit in defining a new kernel mechanism for
exporting the IMA measurement list.

Mimi


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2021-02-09 19:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-05 19:57 [RFC] Persist ima logs to disk Raphael Gianotti
2021-01-07 15:06 ` Mimi Zohar
2021-01-07 16:42   ` James Bottomley
2021-01-07 20:02     ` Mimi Zohar
2021-01-07 20:37       ` James Bottomley
2021-01-07 20:51         ` Mimi Zohar
2021-01-07 21:48           ` James Bottomley
2021-01-07 22:57             ` Raphael Gianotti
2021-01-08 12:38               ` Mimi Zohar
2021-01-08 17:58                 ` Raphael Gianotti
2021-02-01 22:53                   ` Raphael Gianotti
2021-02-02  5:54                     ` Amir Goldstein
2021-02-02 13:07                       ` Mimi Zohar
2021-02-02 18:14                         ` Raphael Gianotti
2021-02-03  1:02                           ` Mimi Zohar
2021-02-03  7:24                             ` Amir Goldstein
2021-02-03 18:45                               ` Mimi Zohar
2021-02-09 17:20                                 ` Raphael Gianotti
2021-02-09 18:08                                   ` Mimi Zohar
2021-01-07 23:00             ` Mimi Zohar
2021-01-07 18:32   ` Raphael Gianotti
2021-01-07 15:45 ` Janne Karhunen
2021-01-07 18:35   ` Raphael Gianotti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).