linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 00/10] ima: carry the measurement list across kexec
@ 2016-10-21  2:44 Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 01/10] powerpc: ima: Get the kexec buffer passed by the previous kernel Thiago Jung Bauermann
                   ` (9 more replies)
  0 siblings, 10 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton,
	Thiago Jung Bauermann

Hello,

This is just a rebase on top of kexec_file_load patches v9 which I just
posted. The previous version of this series has some conflicts with it.

Original cover letter:

The TPM PCRs are only reset on a hard reboot.  In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and then restored on the subsequent
boot, possibly of a different architecture.

The existing securityfs binary_runtime_measurements file conveniently
provides a serialized format of the IMA measurement list. This patch
set serializes the measurement list in this format and restores it.

Up to now, the binary_runtime_measurements was defined as architecture
native format.  The assumption being that userspace could and would
handle any architecture conversions.  With the ability of carrying the
measurement list across kexec, possibly from one architecture to a
different one, the per boot architecture information is lost and with it
the ability of recalculating the template digest hash.  To resolve this
problem, without breaking the existing ABI, this patch set introduces
the boot command line option "ima_canonical_fmt", which is arbitrarily
defined as little endian.

The need for this boot command line option will be limited to the
existing version 1 format of the binary_runtime_measurements.
Subsequent formats will be defined as canonical format (eg. TPM 2.0
support for larger digests).

A simplified method of Thiago Bauermann's "kexec buffer handover" patch
series for carrying the IMA measurement list across kexec is included
in this patch set.  The simplified method requires all file measurements
be taken prior to executing the kexec load, as subsequent measurements
will not be carried across the kexec and restored.

Changelog v6:
- Rebased on top of "kexec_file_load implementation for PowerPC"
  patches v9.

Changelog v5:
- Included patches from Thiago Bauermann's "kexec buffer handover"
patch series for carrying the IMA measurement list across kexec.
- Added CONFIG_HAVE_IMA_KEXEC
- Renamed functions to variations of ima_kexec_buffer instead of
variations of kexec_handover_buffer

Changelog v4:
- Fixed "spinlock bad magic" BUG - reported by Dmitry Vyukov
- Rebased on Thiago Bauermann's v5 patch set
- Removed the skip_checksum initialization  

Changelog v3:
- Cleaned up the code for calculating the requested kexec segment size
needed for the IMA measurement list, limiting the segment size to half
of the totalram_pages.
- Fixed kernel test robot reports as enumerated in the respective
patch changelog.

Changelog v2:
- Canonical measurement list support added
- Redefined the ima_kexec_hdr struct to use well defined sizes

Andreas Steffen (1):
  ima: platform-independent hash value

Mimi Zohar (7):
  ima: on soft reboot, restore the measurement list
  ima: permit duplicate measurement list entries
  ima: maintain memory size needed for serializing the measurement list
  ima: on soft reboot, save the measurement list
  ima: store the builtin/custom template definitions in a list
  ima: support restoring multiple template formats
  ima: define a canonical binary_runtime_measurements list format

Thiago Jung Bauermann (2):
  powerpc: ima: Get the kexec buffer passed by the previous kernel
  powerpc: ima: Send the kexec buffer to the next kernel

 Documentation/kernel-parameters.txt         |   4 +
 arch/Kconfig                                |   3 +
 arch/powerpc/Kconfig                        |   1 +
 arch/powerpc/include/asm/ima.h              |  29 +++
 arch/powerpc/include/asm/kexec.h            |  15 +-
 arch/powerpc/kernel/Makefile                |   4 +
 arch/powerpc/kernel/ima_kexec.c             | 223 +++++++++++++++++++++
 arch/powerpc/kernel/kexec_elf_64.c          |   2 +-
 arch/powerpc/kernel/machine_kexec_file_64.c |  15 +-
 include/linux/ima.h                         |  12 ++
 kernel/kexec_file.c                         |   4 +
 security/integrity/ima/Kconfig              |  12 ++
 security/integrity/ima/Makefile             |   1 +
 security/integrity/ima/ima.h                |  31 +++
 security/integrity/ima/ima_crypto.c         |   6 +-
 security/integrity/ima/ima_fs.c             |  30 ++-
 security/integrity/ima/ima_init.c           |   2 +
 security/integrity/ima/ima_kexec.c          | 168 ++++++++++++++++
 security/integrity/ima/ima_main.c           |   1 +
 security/integrity/ima/ima_queue.c          |  76 +++++++-
 security/integrity/ima/ima_template.c       | 293 ++++++++++++++++++++++++++--
 security/integrity/ima/ima_template_lib.c   |   7 +-
 22 files changed, 901 insertions(+), 38 deletions(-)
 create mode 100644 arch/powerpc/include/asm/ima.h
 create mode 100644 arch/powerpc/kernel/ima_kexec.c
 create mode 100644 security/integrity/ima/ima_kexec.c

-- 
2.7.4

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

* [PATCH v6 01/10] powerpc: ima: Get the kexec buffer passed by the previous kernel
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 02/10] ima: on soft reboot, restore the measurement list Thiago Jung Bauermann
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton,
	Thiago Jung Bauermann

The IMA kexec buffer allows the currently running kernel to pass
the measurement list via a kexec segment to the kernel that will be
kexec'd. The second kernel can check whether the previous kernel sent
the buffer and retrieve it.

This is the architecture-specific part which enables IMA to receive the
measurement list passed by the previous kernel. It will be used in the
next patch.

The change in machine_kexec_64.c is to factor out the logic of removing
an FDT memory reservation so that it can be used by remove_ima_buffer.

Changelog v6:
- The kexec_file_load patches v9 already define delete_fdt_mem_rsv,
  so now we just need to export it.

Changelog v5:
- New patch in this version. This code was previously in the kexec buffer
  handover patch series.

Changelog relative to kexec handover patches v5:
- Added CONFIG_HAVE_IMA_KEXEC.
- Added arch/powerpc/include/asm/ima.h.
- Moved code to arch/powerpc/kernel/ima_kexec.c.
- Renamed functions to variations of ima_kexec_buffer instead of
  variations of kexec_handover_buffer.
- Use a single property /chosen/linux,ima-kexec-buffer containing
  the buffer address and length, instead of
  /chosen/linux,kexec-handover-buffer-{start,end}.
- Use #address-cells and #size-cells to read the DT property.
- Use size_t instead of unsigned long for size arguments.
- Always remove linux,ima-kexec-buffer and its memory reservation
  when preparing a device tree for kexec_file_load.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/Kconfig                                |   3 +
 arch/powerpc/Kconfig                        |   1 +
 arch/powerpc/include/asm/ima.h              |  13 +++
 arch/powerpc/include/asm/kexec.h            |   1 +
 arch/powerpc/kernel/Makefile                |   4 +
 arch/powerpc/kernel/ima_kexec.c             | 132 ++++++++++++++++++++++++++++
 arch/powerpc/kernel/machine_kexec_file_64.c |   5 +-
 7 files changed, 158 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 659bdd079277..e1605ff286a1 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -5,6 +5,9 @@
 config KEXEC_CORE
 	bool
 
+config HAVE_IMA_KEXEC
+	bool
+
 config OPROFILE
 	tristate "OProfile system profiling"
 	depends on PROFILING
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 897d0f14447d..40ee044f1915 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -458,6 +458,7 @@ config KEXEC
 config KEXEC_FILE
 	bool "kexec file based system call"
 	select KEXEC_CORE
+	select HAVE_IMA_KEXEC
 	select BUILD_BIN2C
 	depends on PPC64
 	depends on CRYPTO=y
diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
new file mode 100644
index 000000000000..d5a72dd9b499
--- /dev/null
+++ b/arch/powerpc/include/asm/ima.h
@@ -0,0 +1,13 @@
+#ifndef _ASM_POWERPC_IMA_H
+#define _ASM_POWERPC_IMA_H
+
+int ima_get_kexec_buffer(void **addr, size_t *size);
+int ima_free_kexec_buffer(void);
+
+#ifdef CONFIG_IMA
+void remove_ima_buffer(void *fdt, int chosen_node);
+#else
+static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
+#endif
+
+#endif /* _ASM_POWERPC_IMA_H */
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 4497db7555b0..23056d2dc330 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -101,6 +101,7 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
 int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
 		  unsigned long initrd_len, const char *cmdline);
 bool find_debug_console(const void *fdt);
+int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size);
 #endif /* CONFIG_KEXEC_FILE */
 
 #else /* !CONFIG_KEXEC_CORE */
diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 424b13b1b2b0..c3b37171168c 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -111,6 +111,10 @@ obj-$(CONFIG_KEXEC_CORE)	+= machine_kexec.o crash.o \
 				   machine_kexec_$(BITS).o
 obj-$(CONFIG_KEXEC_FILE)	+= machine_kexec_file_$(BITS).o elf_util.o \
 				   kexec_elf_$(BITS).o
+ifeq ($(CONFIG_HAVE_IMA_KEXEC)$(CONFIG_IMA),yy)
+obj-y				+= ima_kexec.o
+endif
+
 obj-$(CONFIG_AUDIT)		+= audit.o
 obj64-$(CONFIG_AUDIT)		+= compat_audit.o
 
diff --git a/arch/powerpc/kernel/ima_kexec.c b/arch/powerpc/kernel/ima_kexec.c
new file mode 100644
index 000000000000..36e5a5df3804
--- /dev/null
+++ b/arch/powerpc/kernel/ima_kexec.c
@@ -0,0 +1,132 @@
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/slab.h>
+#include <linux/kexec.h>
+#include <linux/of.h>
+#include <linux/memblock.h>
+#include <linux/libfdt.h>
+
+static int get_addr_size_cells(int *addr_cells, int *size_cells)
+{
+	struct device_node *root;
+
+	root = of_find_node_by_path("/");
+	if (!root)
+		return -EINVAL;
+
+	*addr_cells = of_n_addr_cells(root);
+	*size_cells = of_n_size_cells(root);
+
+	of_node_put(root);
+
+	return 0;
+}
+
+static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
+			       size_t *size)
+{
+	int ret, addr_cells, size_cells;
+
+	ret = get_addr_size_cells(&addr_cells, &size_cells);
+	if (ret)
+		return ret;
+
+	if (len < 4 * (addr_cells + size_cells))
+		return -ENOENT;
+
+	*addr = of_read_number(prop, addr_cells);
+	*size = of_read_number(prop + 4 * addr_cells, size_cells);
+
+	return 0;
+}
+
+/**
+ * ima_get_kexec_buffer - get IMA buffer from the previous kernel
+ * @addr:	On successful return, set to point to the buffer contents.
+ * @size:	On successful return, set to the buffer size.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_get_kexec_buffer(void **addr, size_t *size)
+{
+	int ret, len;
+	unsigned long tmp_addr;
+	size_t tmp_size;
+	const void *prop;
+
+	prop = of_get_property(of_chosen, "linux,ima-kexec-buffer", &len);
+	if (!prop)
+		return -ENOENT;
+
+	ret = do_get_kexec_buffer(prop, len, &tmp_addr, &tmp_size);
+	if (ret)
+		return ret;
+
+	*addr = __va(tmp_addr);
+	*size = tmp_size;
+
+	return 0;
+}
+
+/**
+ * ima_free_kexec_buffer - free memory used by the IMA buffer
+ */
+int ima_free_kexec_buffer(void)
+{
+	int ret;
+	unsigned long addr;
+	size_t size;
+	struct property *prop;
+
+	prop = of_find_property(of_chosen, "linux,ima-kexec-buffer", NULL);
+	if (!prop)
+		return -ENOENT;
+
+	ret = do_get_kexec_buffer(prop->value, prop->length, &addr, &size);
+	if (ret)
+		return ret;
+
+	ret = of_remove_property(of_chosen, prop);
+	if (ret)
+		return ret;
+
+	return memblock_free(addr, size);
+
+}
+
+/**
+ * remove_ima_buffer - remove the IMA buffer property and reservation from @fdt
+ *
+ * The IMA measurement buffer is of no use to a subsequent kernel, so we always
+ * remove it from the device tree.
+ */
+void remove_ima_buffer(void *fdt, int chosen_node)
+{
+	int ret, len;
+	unsigned long addr;
+	size_t size;
+	const void *prop;
+
+	prop = fdt_getprop(fdt, chosen_node, "linux,ima-kexec-buffer", &len);
+	if (!prop)
+		return;
+
+	ret = do_get_kexec_buffer(prop, len, &addr, &size);
+	fdt_delprop(fdt, chosen_node, "linux,ima-kexec-buffer");
+	if (ret)
+		return;
+
+	ret = delete_fdt_mem_rsv(fdt, addr, size);
+	if (!ret)
+		pr_debug("Removed old IMA buffer reservation.\n");
+}
diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c b/arch/powerpc/kernel/machine_kexec_file_64.c
index 9acc56d199f0..7b13634f71bf 100644
--- a/arch/powerpc/kernel/machine_kexec_file_64.c
+++ b/arch/powerpc/kernel/machine_kexec_file_64.c
@@ -28,6 +28,7 @@
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
 #include <asm/elf_util.h>
+#include <asm/ima.h>
 
 #define SLAVE_CODE_SIZE		256
 
@@ -363,7 +364,7 @@ int setup_purgatory(struct kimage *image, const void *slave_code,
  *
  * Return: 0 on success, or negative errno on error.
  */
-static int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
+int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
 {
 	int i, ret, num_rsvs = fdt_num_mem_rsv(fdt);
 
@@ -511,6 +512,8 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
 		}
 	}
 
+	remove_ima_buffer(fdt, chosen_node);
+
 	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
 	if (ret) {
 		pr_err("Error setting up the new device tree.\n");
-- 
2.7.4

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

* [PATCH v6 02/10] ima: on soft reboot, restore the measurement list
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 01/10] powerpc: ima: Get the kexec buffer passed by the previous kernel Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-11-08 19:46   ` [Linux-ima-devel] " Dmitry Kasatkin
  2016-10-21  2:44 ` [PATCH v6 03/10] ima: permit duplicate measurement list entries Thiago Jung Bauermann
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

The TPM PCRs are only reset on a hard reboot.  In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot.  This patch
restores the measurement list.

Changelog v5:
- replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
- replace kexec_get_handover_buffer() with ima_get_kexec_buffer() (Thiago)
- replace kexec_free_handover_buffer() with ima_free_kexec_buffer() (Thiago)
- remove unnecessary includes from ima_kexec.c (Thiago)
- fix off-by-one error when checking hdr_v1->template_name_len (Colin King)

Changelog v2:
- redefined ima_kexec_hdr to use types with well defined sizes (M. Ellerman)
- defined missing ima_load_kexec_buffer() stub function

Changelog v1:
- call ima_load_kexec_buffer() (Thiago)

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Makefile       |   1 +
 security/integrity/ima/ima.h          |  21 +++++
 security/integrity/ima/ima_init.c     |   2 +
 security/integrity/ima/ima_kexec.c    |  44 +++++++++
 security/integrity/ima/ima_queue.c    |  10 ++
 security/integrity/ima/ima_template.c | 170 ++++++++++++++++++++++++++++++++++
 6 files changed, 248 insertions(+)

diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
index 9aeaedad1e2b..29f198bde02b 100644
--- a/security/integrity/ima/Makefile
+++ b/security/integrity/ima/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_IMA) += ima.o
 ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
 	 ima_policy.o ima_template.o ima_template_lib.o
 ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
+ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
 obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index db25f54a04fe..51dc8d57d64d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -28,6 +28,10 @@
 
 #include "../integrity.h"
 
+#ifdef CONFIG_HAVE_IMA_KEXEC
+#include <asm/ima.h>
+#endif
+
 enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
 		     IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
 enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
@@ -102,6 +106,21 @@ struct ima_queue_entry {
 };
 extern struct list_head ima_measurements;	/* list of all measurements */
 
+/* Some details preceding the binary serialized measurement list */
+struct ima_kexec_hdr {
+	u16 version;
+	u16 _reserved0;
+	u32 _reserved1;
+	u64 buffer_size;
+	u64 count;
+};
+
+#ifdef CONFIG_HAVE_IMA_KEXEC
+void ima_load_kexec_buffer(void);
+#else
+static inline void ima_load_kexec_buffer(void) {}
+#endif /* CONFIG_HAVE_IMA_KEXEC */
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
@@ -122,6 +141,8 @@ int ima_init_crypto(void);
 void ima_putc(struct seq_file *m, void *data, int datalen);
 void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
+int ima_restore_measurement_entry(struct ima_template_entry *entry);
+int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_init_template(void);
 
 /*
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 32912bd54ead..3ba0ca49cba6 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -128,6 +128,8 @@ int __init ima_init(void)
 	if (rc != 0)
 		return rc;
 
+	ima_load_kexec_buffer();
+
 	rc = ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	if (rc != 0)
 		return rc;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
new file mode 100644
index 000000000000..36afd0fe9747
--- /dev/null
+++ b/security/integrity/ima/ima_kexec.c
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2016 IBM Corporation
+ *
+ * Authors:
+ * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
+ * Mimi Zohar <zohar@linux.vnet.ibm.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+#include "ima.h"
+
+/*
+ * Restore the measurement list from the previous kernel.
+ */
+void ima_load_kexec_buffer(void)
+{
+	void *kexec_buffer = NULL;
+	size_t kexec_buffer_size = 0;
+	int rc;
+
+	rc = ima_get_kexec_buffer(&kexec_buffer, &kexec_buffer_size);
+	switch (rc) {
+	case 0:
+		rc = ima_restore_measurement_list(kexec_buffer_size,
+						  kexec_buffer);
+		if (rc != 0)
+			pr_err("Failed to restore the measurement list: %d\n",
+				rc);
+
+		ima_free_kexec_buffer();
+		break;
+	case -ENOTSUPP:
+		pr_debug("Restoring the measurement list not supported\n");
+		break;
+	case -ENOENT:
+		pr_debug("No measurement list to restore\n");
+		break;
+	default:
+		pr_debug("Error restoring the measurement list: %d\n", rc);
+	}
+}
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 32f6ac0f96df..4b1bb7787839 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -149,3 +149,13 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			    op, audit_cause, result, audit_info);
 	return result;
 }
+
+int ima_restore_measurement_entry(struct ima_template_entry *entry)
+{
+	int result = 0;
+
+	mutex_lock(&ima_extend_list_mutex);
+	result = ima_add_digest_entry(entry);
+	mutex_unlock(&ima_extend_list_mutex);
+	return result;
+}
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index febd12ed9b55..37f972cb05fe 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -37,6 +37,7 @@ static struct ima_template_field supported_fields[] = {
 	{.field_id = "sig", .field_init = ima_eventsig_init,
 	 .field_show = ima_show_template_sig},
 };
+#define MAX_TEMPLATE_NAME_LEN 15
 
 static struct ima_template_desc *ima_template;
 static struct ima_template_desc *lookup_template_desc(const char *name);
@@ -205,3 +206,172 @@ int __init ima_init_template(void)
 
 	return result;
 }
+
+static int ima_restore_template_data(struct ima_template_desc *template_desc,
+				     void *template_data,
+				     int template_data_size,
+				     struct ima_template_entry **entry)
+{
+	struct binary_field_data {
+		u32 len;
+		u8 data[0];
+	} __packed;
+
+	struct binary_field_data *field_data;
+	int offset = 0;
+	int ret = 0;
+	int i;
+
+	*entry = kzalloc(sizeof(**entry) +
+		    template_desc->num_fields * sizeof(struct ima_field_data),
+		    GFP_NOFS);
+	if (!*entry)
+		return -ENOMEM;
+
+	(*entry)->template_desc = template_desc;
+	for (i = 0; i < template_desc->num_fields; i++) {
+		field_data = template_data + offset;
+
+		/* Each field of the template data is prefixed with a length. */
+		if (offset > (template_data_size - sizeof(field_data->len))) {
+			pr_err("Restoring the template field failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		offset += sizeof(field_data->len);
+
+		if (offset > (template_data_size - field_data->len)) {
+			pr_err("Restoring the template field data failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		offset += field_data->len;
+
+		(*entry)->template_data[i].len = field_data->len;
+		(*entry)->template_data_len += sizeof(field_data->len);
+
+		(*entry)->template_data[i].data =
+			kzalloc(field_data->len + 1, GFP_KERNEL);
+		if (!(*entry)->template_data[i].data) {
+			ret = -ENOMEM;
+			break;
+		}
+		memcpy((*entry)->template_data[i].data, field_data->data,
+			field_data->len);
+		(*entry)->template_data_len += field_data->len;
+	}
+
+	if (ret < 0) {
+		ima_free_template_entry(*entry);
+		*entry = NULL;
+	}
+
+	return ret;
+}
+
+/* Restore the serialized binary measurement list without extending PCRs. */
+int ima_restore_measurement_list(loff_t size, void *buf)
+{
+	struct binary_hdr_v1 {
+		u32 pcr;
+		u8 digest[TPM_DIGEST_SIZE];
+		u32 template_name_len;
+		char template_name[0];
+	} __packed;
+	char template_name[MAX_TEMPLATE_NAME_LEN];
+
+	struct binary_data_v1 {
+		u32 template_data_size;
+		char template_data[0];
+	} __packed;
+
+	struct ima_kexec_hdr *khdr = buf;
+	struct binary_hdr_v1 *hdr_v1;
+	struct binary_data_v1 *data_v1;
+
+	void *bufp = buf + sizeof(*khdr);
+	void *bufendp = buf + khdr->buffer_size;
+	struct ima_template_entry *entry;
+	struct ima_template_desc *template_desc;
+	unsigned long count = 0;
+	int ret = 0;
+
+	if (!buf || size < sizeof(*khdr))
+		return 0;
+
+	if (khdr->version != 1) {
+		pr_err("attempting to restore a incompatible measurement list");
+		return 0;
+	}
+
+	/*
+	 * ima kexec buffer prefix: version, buffer size, count
+	 * v1 format: pcr, digest, template-name-len, template-name,
+	 *	      template-data-size, template-data
+	 */
+	while ((bufp < bufendp) && (count++ < khdr->count)) {
+		if (count > ULONG_MAX - 1) {
+			pr_err("attempting to restore too many measurements");
+			ret = -EINVAL;
+		}
+
+		hdr_v1 = bufp;
+		if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) ||
+		    ((bufp + hdr_v1->template_name_len) > bufendp)) {
+			pr_err("attempting to restore a template name \
+				that is too long\n");
+			ret = -EINVAL;
+			break;
+		}
+		bufp += sizeof(*hdr_v1);
+
+		/* template name is not null terminated */
+		memcpy(template_name, bufp, hdr_v1->template_name_len);
+		template_name[hdr_v1->template_name_len] = 0;
+
+		if (strcmp(template_name, "ima") == 0) {
+			pr_err("attempting to restore an unsupported \
+				template \"%s\" failed\n", template_name);
+			ret = -EINVAL;
+			break;
+		}
+		data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
+
+		/* get template format */
+		template_desc = lookup_template_desc(template_name);
+		if (!template_desc) {
+			pr_err("template \"%s\" not found\n", template_name);
+			ret = -EINVAL;
+			break;
+		}
+
+		if (bufp > (bufendp - sizeof(data_v1->template_data_size))) {
+			pr_err("restoring the template data size failed\n");
+			ret = -EINVAL;
+			break;
+		}
+		bufp += (u_int8_t) sizeof(data_v1->template_data_size);
+
+		if (bufp > (bufendp - data_v1->template_data_size)) {
+			pr_err("restoring the template data failed\n");
+			ret = -EINVAL;
+			break;
+		}
+
+		ret = ima_restore_template_data(template_desc,
+						data_v1->template_data,
+						data_v1->template_data_size,
+						&entry);
+		if (ret < 0)
+			break;
+
+		memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
+		entry->pcr = hdr_v1->pcr;
+		ret = ima_restore_measurement_entry(entry);
+		if (ret < 0)
+			break;
+
+		bufp += data_v1->template_data_size;
+	}
+	return ret;
+}
-- 
2.7.4

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

* [PATCH v6 03/10] ima: permit duplicate measurement list entries
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 01/10] powerpc: ima: Get the kexec buffer passed by the previous kernel Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 02/10] ima: on soft reboot, restore the measurement list Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-11-08 19:47   ` [Linux-ima-devel] " Dmitry Kasatkin
  2016-10-21  2:44 ` [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list Thiago Jung Bauermann
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

Measurements carried across kexec need to be added to the IMA
measurement list, but should not prevent measurements of the newly
booted kernel from being added to the measurement list. This patch
adds support for allowing duplicate measurements.

The "boot_aggregate" measurement entry is the delimiter between soft
boots.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_queue.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 4b1bb7787839..12d1b040bca9 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -65,11 +65,12 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 }
 
 /* ima_add_template_entry helper function:
- * - Add template entry to measurement list and hash table.
+ * - Add template entry to the measurement list and hash table, for
+ *   all entries except those carried across kexec.
  *
  * (Called with ima_extend_list_mutex held.)
  */
-static int ima_add_digest_entry(struct ima_template_entry *entry)
+static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
 {
 	struct ima_queue_entry *qe;
 	unsigned int key;
@@ -85,8 +86,10 @@ static int ima_add_digest_entry(struct ima_template_entry *entry)
 	list_add_tail_rcu(&qe->later, &ima_measurements);
 
 	atomic_long_inc(&ima_htable.len);
-	key = ima_hash_key(entry->digest);
-	hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
+	if (flags) {
+		key = ima_hash_key(entry->digest);
+		hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
+	}
 	return 0;
 }
 
@@ -126,7 +129,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 		}
 	}
 
-	result = ima_add_digest_entry(entry);
+	result = ima_add_digest_entry(entry, 1);
 	if (result < 0) {
 		audit_cause = "ENOMEM";
 		audit_info = 0;
@@ -155,7 +158,7 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)
 	int result = 0;
 
 	mutex_lock(&ima_extend_list_mutex);
-	result = ima_add_digest_entry(entry);
+	result = ima_add_digest_entry(entry, 0);
 	mutex_unlock(&ima_extend_list_mutex);
 	return result;
 }
-- 
2.7.4

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

* [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (2 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 03/10] ima: permit duplicate measurement list entries Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-11-08 20:05   ` [Linux-ima-devel] " Dmitry Kasatkin
  2016-10-21  2:44 ` [PATCH v6 05/10] powerpc: ima: Send the kexec buffer to the next kernel Thiago Jung Bauermann
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

In preparation for serializing the binary_runtime_measurements, this patch
maintains the amount of memory required.

Changelog v5:
- replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)

Changelog v3:
- include the ima_kexec_hdr size in the binary_runtime_measurement size.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/Kconfig     | 12 +++++++++
 security/integrity/ima/ima.h       |  1 +
 security/integrity/ima/ima_queue.c | 53 ++++++++++++++++++++++++++++++++++++--
 3 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 5487827fa86c..370eb2f4dd37 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -27,6 +27,18 @@ config IMA
 	  to learn more about IMA.
 	  If unsure, say N.
 
+config IMA_KEXEC
+	bool "Enable carrying the IMA measurement list across a soft boot"
+	depends on IMA && TCG_TPM && HAVE_IMA_KEXEC
+	default n
+	help
+	   TPM PCRs are only reset on a hard reboot.  In order to validate
+	   a TPM's quote after a soft boot, the IMA measurement list of the
+	   running kernel must be saved and restored on boot.
+
+	   Depending on the IMA policy, the measurement list can grow to
+	   be very large.
+
 config IMA_MEASURE_PCR_IDX
 	int
 	depends on IMA
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 51dc8d57d64d..ea1dcc452911 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -143,6 +143,7 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
 int ima_restore_measurement_entry(struct ima_template_entry *entry);
 int ima_restore_measurement_list(loff_t bufsize, void *buf);
+unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
 
 /*
diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
index 12d1b040bca9..3a3cc2a45645 100644
--- a/security/integrity/ima/ima_queue.c
+++ b/security/integrity/ima/ima_queue.c
@@ -29,6 +29,11 @@
 #define AUDIT_CAUSE_LEN_MAX 32
 
 LIST_HEAD(ima_measurements);	/* list of all measurements */
+#ifdef CONFIG_IMA_KEXEC
+static unsigned long binary_runtime_size;
+#else
+static unsigned long binary_runtime_size = ULONG_MAX;
+#endif
 
 /* key: inode (before secure-hashing a file) */
 struct ima_h_table ima_htable = {
@@ -64,6 +69,24 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
 	return ret;
 }
 
+/*
+ * Calculate the memory required for serializing a single
+ * binary_runtime_measurement list entry, which contains a
+ * couple of variable length fields (e.g template name and data).
+ */
+static int get_binary_runtime_size(struct ima_template_entry *entry)
+{
+	int size = 0;
+
+	size += sizeof(u32);	/* pcr */
+	size += sizeof(entry->digest);
+	size += sizeof(int);	/* template name size field */
+	size += strlen(entry->template_desc->name);
+	size += sizeof(entry->template_data_len);
+	size += entry->template_data_len;
+	return size;
+}
+
 /* ima_add_template_entry helper function:
  * - Add template entry to the measurement list and hash table, for
  *   all entries except those carried across kexec.
@@ -90,9 +113,30 @@ static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
 		key = ima_hash_key(entry->digest);
 		hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
 	}
+
+	if (binary_runtime_size != ULONG_MAX) {
+		int size;
+
+		size = get_binary_runtime_size(entry);
+		binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
+		     binary_runtime_size + size : ULONG_MAX;
+	}
 	return 0;
 }
 
+/*
+ * Return the amount of memory required for serializing the
+ * entire binary_runtime_measurement list, including the ima_kexec_hdr
+ * structure.
+ */
+unsigned long ima_get_binary_runtime_size(void)
+{
+	if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
+		return ULONG_MAX;
+	else
+		return binary_runtime_size + sizeof(struct ima_kexec_hdr);
+};
+
 static int ima_pcr_extend(const u8 *hash, int pcr)
 {
 	int result = 0;
@@ -106,8 +150,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
 	return result;
 }
 
-/* Add template entry to the measurement list and hash table,
- * and extend the pcr.
+/*
+ * Add template entry to the measurement list and hash table, and
+ * extend the pcr.
+ *
+ * On systems which support carrying the IMA measurement list across
+ * kexec, maintain the total memory size required for serializing the
+ * binary_runtime_measurements.
  */
 int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
-- 
2.7.4

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

* [PATCH v6 05/10] powerpc: ima: Send the kexec buffer to the next kernel
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (3 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 06/10] ima: on soft reboot, save the measurement list Thiago Jung Bauermann
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton,
	Thiago Jung Bauermann

The IMA kexec buffer allows the currently running kernel to pass
the measurement list via a kexec segment to the kernel that will be
kexec'd.

This is the architecture-specific part of setting up the IMA kexec
buffer for the next kernel. It will be used in the next patch.

Changelog v5:
- New patch in this version. This code was previously in the kexec buffer
  handover patch series.

Changelog relative to kexec handover patches v5:
- Moved code to arch/powerpc/kernel/ima_kexec.c.
- Renamed functions and struct members to variations of ima_kexec_buffer
  instead of variations of kexec_handover_buffer.
- Use a single property /chosen/linux,ima-kexec-buffer containing
  the buffer address and length, instead of
  /chosen/linux,kexec-handover-buffer-{start,end}.
- Use #address-cells and #size-cells to write the DT property.
- Use size_t instead of unsigned long for size arguments.
- Use CONFIG_IMA_KEXEC to build this code only when necessary.

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/powerpc/include/asm/ima.h              | 16 +++++
 arch/powerpc/include/asm/kexec.h            | 14 ++++-
 arch/powerpc/kernel/ima_kexec.c             | 91 +++++++++++++++++++++++++++++
 arch/powerpc/kernel/kexec_elf_64.c          |  2 +-
 arch/powerpc/kernel/machine_kexec_file_64.c | 12 +++-
 5 files changed, 129 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
index d5a72dd9b499..2313bdface34 100644
--- a/arch/powerpc/include/asm/ima.h
+++ b/arch/powerpc/include/asm/ima.h
@@ -1,6 +1,8 @@
 #ifndef _ASM_POWERPC_IMA_H
 #define _ASM_POWERPC_IMA_H
 
+struct kimage;
+
 int ima_get_kexec_buffer(void **addr, size_t *size);
 int ima_free_kexec_buffer(void);
 
@@ -10,4 +12,18 @@ void remove_ima_buffer(void *fdt, int chosen_node);
 static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
 #endif
 
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      size_t size);
+
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
+#else
+static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
+				   int chosen_node)
+{
+	remove_ima_buffer(fdt, chosen_node);
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+
 #endif /* _ASM_POWERPC_IMA_H */
diff --git a/arch/powerpc/include/asm/kexec.h b/arch/powerpc/include/asm/kexec.h
index 23056d2dc330..a49cab287acb 100644
--- a/arch/powerpc/include/asm/kexec.h
+++ b/arch/powerpc/include/asm/kexec.h
@@ -94,12 +94,22 @@ static inline bool kdump_in_progress(void)
 #ifdef CONFIG_KEXEC_FILE
 extern struct kexec_file_ops kexec_elf64_ops;
 
+#ifdef CONFIG_IMA_KEXEC
+#define ARCH_HAS_KIMAGE_ARCH
+
+struct kimage_arch {
+	phys_addr_t ima_buffer_addr;
+	size_t ima_buffer_size;
+};
+#endif
+
 int setup_purgatory(struct kimage *image, const void *slave_code,
 		    const void *fdt, unsigned long kernel_load_addr,
 		    unsigned long fdt_load_addr, unsigned long stack_top,
 		    int debug);
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline);
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline);
 bool find_debug_console(const void *fdt);
 int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size);
 #endif /* CONFIG_KEXEC_FILE */
diff --git a/arch/powerpc/kernel/ima_kexec.c b/arch/powerpc/kernel/ima_kexec.c
index 36e5a5df3804..5ea42c937ca9 100644
--- a/arch/powerpc/kernel/ima_kexec.c
+++ b/arch/powerpc/kernel/ima_kexec.c
@@ -130,3 +130,94 @@ void remove_ima_buffer(void *fdt, int chosen_node)
 	if (!ret)
 		pr_debug("Removed old IMA buffer reservation.\n");
 }
+
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
+ *
+ * Architectures should use this function to pass on the IMA buffer
+ * information to the next kernel.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      size_t size)
+{
+	image->arch.ima_buffer_addr = load_addr;
+	image->arch.ima_buffer_size = size;
+
+	return 0;
+}
+
+static int write_number(void *p, u64 value, int cells)
+{
+	if (cells == 1) {
+		u32 tmp;
+
+		if (value > U32_MAX)
+			return -EINVAL;
+
+		tmp = cpu_to_be32(value);
+		memcpy(p, &tmp, sizeof(tmp));
+	} else if (cells == 2) {
+		u64 tmp;
+
+		tmp = cpu_to_be64(value);
+		memcpy(p, &tmp, sizeof(tmp));
+	} else
+		return -EINVAL;
+
+	return 0;
+}
+
+/**
+ * setup_ima_buffer - add IMA buffer information to the fdt
+ * @image:		kexec image being loaded.
+ * @fdt:		Flattened device tree for the next kernel.
+ * @chosen_node:	Offset to the chosen node.
+ *
+ * Return: 0 on success, or negative errno on error.
+ */
+int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node)
+{
+	int ret, addr_cells, size_cells, entry_size;
+	u8 value[16];
+
+	remove_ima_buffer(fdt, chosen_node);
+	if (!image->arch.ima_buffer_size)
+		return 0;
+
+	ret = get_addr_size_cells(&addr_cells, &size_cells);
+	if (ret)
+		return ret;
+
+	entry_size = 4 * (addr_cells + size_cells);
+
+	if (entry_size > sizeof(value))
+		return -EINVAL;
+
+	ret = write_number(value, image->arch.ima_buffer_addr, addr_cells);
+	if (ret)
+		return ret;
+
+	ret = write_number(value + 4 * addr_cells, image->arch.ima_buffer_size,
+			   size_cells);
+	if (ret)
+		return ret;
+
+	ret = fdt_setprop(fdt, chosen_node, "linux,ima-kexec-buffer", value,
+			  entry_size);
+	if (ret < 0)
+		return -EINVAL;
+
+	ret = fdt_add_mem_rsv(fdt, image->arch.ima_buffer_addr,
+			      image->arch.ima_buffer_size);
+	if (ret)
+		return -EINVAL;
+
+	pr_debug("IMA buffer at 0x%llx, size = 0x%zx\n",
+		 image->arch.ima_buffer_addr, image->arch.ima_buffer_size);
+
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
diff --git a/arch/powerpc/kernel/kexec_elf_64.c b/arch/powerpc/kernel/kexec_elf_64.c
index dc29e0131b76..1bd1e9865835 100644
--- a/arch/powerpc/kernel/kexec_elf_64.c
+++ b/arch/powerpc/kernel/kexec_elf_64.c
@@ -206,7 +206,7 @@ static void *elf64_load(struct kimage *image, char *kernel_buf,
 		goto out;
 	}
 
-	ret = setup_new_fdt(fdt, initrd_load_addr, initrd_len, cmdline);
+	ret = setup_new_fdt(image, fdt, initrd_load_addr, initrd_len, cmdline);
 	if (ret)
 		goto out;
 
diff --git a/arch/powerpc/kernel/machine_kexec_file_64.c b/arch/powerpc/kernel/machine_kexec_file_64.c
index 7b13634f71bf..e4da26dabbaf 100644
--- a/arch/powerpc/kernel/machine_kexec_file_64.c
+++ b/arch/powerpc/kernel/machine_kexec_file_64.c
@@ -393,6 +393,7 @@ int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
 
 /*
  * setup_new_fdt - modify /chosen and memory reservation for the next kernel
+ * @image:		kexec image being loaded.
  * @fdt:		Flattened device tree for the next kernel.
  * @initrd_load_addr:	Address where the next initrd will be loaded.
  * @initrd_len:		Size of the next initrd, or 0 if there will be none.
@@ -401,8 +402,9 @@ int delete_fdt_mem_rsv(void *fdt, unsigned long start, unsigned long size)
  *
  * Return: 0 on success, or negative errno on error.
  */
-int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
-		  unsigned long initrd_len, const char *cmdline)
+int setup_new_fdt(const struct kimage *image, void *fdt,
+		  unsigned long initrd_load_addr, unsigned long initrd_len,
+		  const char *cmdline)
 {
 	int ret, chosen_node;
 	const void *prop;
@@ -512,7 +514,11 @@ int setup_new_fdt(void *fdt, unsigned long initrd_load_addr,
 		}
 	}
 
-	remove_ima_buffer(fdt, chosen_node);
+	ret = setup_ima_buffer(image, fdt, chosen_node);
+	if (ret) {
+		pr_err("Error setting up the new device tree.\n");
+		return ret;
+	}
 
 	ret = fdt_setprop(fdt, chosen_node, "linux,booted-from-kexec", NULL, 0);
 	if (ret) {
-- 
2.7.4

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

* [PATCH v6 06/10] ima: on soft reboot, save the measurement list
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (4 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 05/10] powerpc: ima: Send the kexec buffer to the next kernel Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list Thiago Jung Bauermann
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton,
	Thiago Jung Bauermann

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

The TPM PCRs are only reset on a hard reboot.  In order to validate a
TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
of the running kernel must be saved and restored on boot.

This patch uses the kexec buffer passing mechanism to pass the
serialized IMA binary_runtime_measurements to the next kernel.

Changelog v5:
- move writing the IMA measurement list to kexec load and remove
  from kexec execute.
- remove registering notifier to call update on kexec execute
- add includes needed by code in this patch to ima_kexec.c (Thiago)
- fold patch "ima: serialize the binary_runtime_measurements"
into this patch.

Changelog v4:
- Revert the skip_checksum change.  Instead calculate the checksum
with the measurement list segment, on update validate the existing
checksum before re-calulating a new checksum with the updated
measurement list.

Changelog v3:
- Request a kexec segment for storing the measurement list a half page,
not a full page, more than needed for additional measurements.
- Added binary_runtime_size overflow test
- Limit maximum number of pages needed for kexec_segment_size to half
of totalram_pages. (Dave Young)

Changelog v2:
- Fix build issue by defining a stub ima_add_kexec_buffer and stub
  struct kimage when CONFIG_IMA=n and CONFIG_IMA_KEXEC=n. (Fenguang Wu)
- removed kexec_add_handover_buffer() checksum argument.
- added skip_checksum member to kexec_buf
- only register reboot notifier once

Changelog v1:
- updated to call IMA functions  (Mimi)
- move code from ima_template.c to ima_kexec.c (Mimi)

Signed-off-by: Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/linux/ima.h                |  12 ++++
 kernel/kexec_file.c                |   4 ++
 security/integrity/ima/ima.h       |   1 +
 security/integrity/ima/ima_fs.c    |   2 +-
 security/integrity/ima/ima_kexec.c | 117 +++++++++++++++++++++++++++++++++++++
 5 files changed, 135 insertions(+), 1 deletion(-)

diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0eb7c2e7f0d6..7f6952f8d6aa 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -11,6 +11,7 @@
 #define _LINUX_IMA_H
 
 #include <linux/fs.h>
+#include <linux/kexec.h>
 struct linux_binprm;
 
 #ifdef CONFIG_IMA
@@ -23,6 +24,10 @@ extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
 			      enum kernel_read_file_id id);
 extern void ima_post_path_mknod(struct dentry *dentry);
 
+#ifdef CONFIG_IMA_KEXEC
+extern void ima_add_kexec_buffer(struct kimage *image);
+#endif
+
 #else
 static inline int ima_bprm_check(struct linux_binprm *bprm)
 {
@@ -62,6 +67,13 @@ static inline void ima_post_path_mknod(struct dentry *dentry)
 
 #endif /* CONFIG_IMA */
 
+#ifndef CONFIG_IMA_KEXEC
+struct kimage;
+
+static inline void ima_add_kexec_buffer(struct kimage *image)
+{}
+#endif
+
 #ifdef CONFIG_IMA_APPRAISE
 extern void ima_inode_post_setattr(struct dentry *dentry);
 extern int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name,
diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 0c2df7f73792..b56a558e406d 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -19,6 +19,7 @@
 #include <linux/mutex.h>
 #include <linux/list.h>
 #include <linux/fs.h>
+#include <linux/ima.h>
 #include <crypto/hash.h>
 #include <crypto/sha.h>
 #include <linux/syscalls.h>
@@ -132,6 +133,9 @@ kimage_file_prepare_segments(struct kimage *image, int kernel_fd, int initrd_fd,
 		return ret;
 	image->kernel_buf_len = size;
 
+	/* IMA needs to pass the measurement list to the next kernel. */
+	ima_add_kexec_buffer(image);
+
 	/* Call arch image probe handlers */
 	ret = arch_kexec_kernel_image_probe(image, image->kernel_buf,
 					    image->kernel_buf_len);
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index ea1dcc452911..139dec67dcbf 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -143,6 +143,7 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
 struct ima_template_desc *ima_template_desc_current(void);
 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);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index c07a3844ea0a..66e5dd5e226f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -116,7 +116,7 @@ void ima_putc(struct seq_file *m, void *data, int datalen)
  *       [eventdata length]
  *       eventdata[n]=template specific data
  */
-static int ima_measurements_show(struct seq_file *m, void *v)
+int ima_measurements_show(struct seq_file *m, void *v)
 {
 	/* the list never shrinks, so we don't need a lock here */
 	struct ima_queue_entry *qe = v;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 36afd0fe9747..2c4824ac1ce1 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -10,8 +10,125 @@
  * the Free Software Foundation; either version 2 of the License, or
  * (at your option) any later version.
  */
+#include <linux/seq_file.h>
+#include <linux/vmalloc.h>
+#include <linux/kexec.h>
 #include "ima.h"
 
+#ifdef CONFIG_IMA_KEXEC
+static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
+				     unsigned long segment_size)
+{
+	struct ima_queue_entry *qe;
+	struct seq_file file;
+	struct ima_kexec_hdr khdr = {
+		.version = 1, .buffer_size = 0, .count = 0};
+	int ret = 0;
+
+	/* segment size can't change between kexec load and execute */
+	file.buf = vmalloc(segment_size);
+	if (!file.buf) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	file.size = segment_size;
+	file.read_pos = 0;
+	file.count = sizeof(khdr);	/* reserved space */
+
+	list_for_each_entry_rcu(qe, &ima_measurements, later) {
+		if (file.count < file.size) {
+			khdr.count++;
+			ima_measurements_show(&file, qe);
+		} else {
+			ret = -EINVAL;
+			break;
+		}
+	}
+
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * fill in reserved space with some buffer details
+	 * (eg. version, buffer size, number of measurements)
+	 */
+	khdr.buffer_size = file.count;
+	memcpy(file.buf, &khdr, sizeof(khdr));
+	print_hex_dump(KERN_DEBUG, "ima dump: ", DUMP_PREFIX_NONE,
+			16, 1, file.buf,
+			file.count < 100 ? file.count : 100, true);
+
+	*buffer_size = file.count;
+	*buffer = file.buf;
+out:
+	if (ret == -EINVAL)
+		vfree(file.buf);
+	return ret;
+}
+
+/*
+ * Called during kexec_file_load so that IMA can add a segment to the kexec
+ * image for the measurement list for the next kernel.
+ *
+ * This function assumes that kexec_mutex is held.
+ */
+void ima_add_kexec_buffer(struct kimage *image)
+{
+	struct kexec_buf kbuf = { .image = image, .buf_align = PAGE_SIZE,
+				  .buf_min = 0, .buf_max = ULONG_MAX,
+				  .top_down = true };
+	unsigned long binary_runtime_size;
+
+	/* use more understandable variable names than defined in kbuf */
+	void *kexec_buffer = NULL;
+	size_t kexec_buffer_size;
+	size_t kexec_segment_size;
+	int ret;
+
+	/*
+	 * Reserve an extra half page of memory for additional measurements
+	 * added during the kexec load.
+	 */
+	binary_runtime_size = ima_get_binary_runtime_size();
+	if (binary_runtime_size >= ULONG_MAX - PAGE_SIZE)
+		kexec_segment_size = ULONG_MAX;
+	else
+		kexec_segment_size = ALIGN(ima_get_binary_runtime_size() +
+					   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;
+	}
+
+	ima_dump_measurement_list(&kexec_buffer_size, &kexec_buffer,
+				  kexec_segment_size);
+	if (!kexec_buffer) {
+		pr_err("Not enough memory for the kexec measurement buffer.\n");
+		return;
+	}
+
+	kbuf.buffer = kexec_buffer;
+	kbuf.bufsz = kexec_buffer_size;
+	kbuf.memsz = kexec_segment_size;
+	ret = kexec_add_buffer(&kbuf);
+	if (ret) {
+		pr_err("Error passing over kexec measurement buffer.\n");
+		return;
+	}
+
+	ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_segment_size);
+	if (ret) {
+		pr_err("Error passing over kexec measurement buffer.\n");
+		return;
+	}
+
+	pr_debug("kexec measurement buffer for the loaded kernel at 0x%lx.\n",
+		 kbuf.mem);
+}
+#endif /* IMA_KEXEC */
+
 /*
  * Restore the measurement list from the previous kernel.
  */
-- 
2.7.4

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

* [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (5 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 06/10] ima: on soft reboot, save the measurement list Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-11-08 23:40   ` [Linux-ima-devel] " Dmitry Kasatkin
  2016-10-21  2:44 ` [PATCH v6 08/10] ima: support restoring multiple template formats Thiago Jung Bauermann
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

The builtin and single custom templates are currently stored in an
array.  In preparation for being able to restore a measurement list
containing multiple builtin/custom templates, this patch stores the
builtin and custom templates as a linked list.  This will permit
defining more than one custom template per boot.

Changelog v4:
- fix "spinlock bad magic" BUG - reported by Dmitry Vyukov

Changelog v3:
- initialize template format list in ima_template_desc_current(), as it
might be called during __setup before normal initialization. (kernel
test robot)
- remove __init annotation of ima_init_template_list()

Changelog v2:
- fix lookup_template_desc() preemption imbalance (kernel test robot)

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima.h          |  2 ++
 security/integrity/ima/ima_main.c     |  1 +
 security/integrity/ima/ima_template.c | 52 +++++++++++++++++++++++++++--------
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 139dec67dcbf..6b0540ad189f 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -85,6 +85,7 @@ struct ima_template_field {
 
 /* IMA template descriptor definition */
 struct ima_template_desc {
+	struct list_head list;
 	char *name;
 	char *fmt;
 	int num_fields;
@@ -146,6 +147,7 @@ int ima_restore_measurement_list(loff_t bufsize, void *buf);
 int ima_measurements_show(struct seq_file *m, void *v);
 unsigned long ima_get_binary_runtime_size(void);
 int ima_init_template(void);
+void ima_init_template_list(void);
 
 /*
  * used to protect h_table and sha_table
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 423d111b3b94..50818c60538b 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -418,6 +418,7 @@ static int __init init_ima(void)
 {
 	int error;
 
+	ima_init_template_list();
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
 	error = ima_init();
 	if (!error) {
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index 37f972cb05fe..c0d808c20c40 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -15,16 +15,20 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/rculist.h>
 #include "ima.h"
 #include "ima_template_lib.h"
 
-static struct ima_template_desc defined_templates[] = {
+static struct ima_template_desc builtin_templates[] = {
 	{.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
 	{.name = "ima-ng", .fmt = "d-ng|n-ng"},
 	{.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
 	{.name = "", .fmt = ""},	/* placeholder for a custom format */
 };
 
+static LIST_HEAD(defined_templates);
+static DEFINE_SPINLOCK(template_list);
+
 static struct ima_template_field supported_fields[] = {
 	{.field_id = "d", .field_init = ima_eventdigest_init,
 	 .field_show = ima_show_template_digest},
@@ -53,6 +57,8 @@ static int __init ima_template_setup(char *str)
 	if (ima_template)
 		return 1;
 
+	ima_init_template_list();
+
 	/*
 	 * Verify that a template with the supplied name exists.
 	 * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
@@ -81,7 +87,7 @@ __setup("ima_template=", ima_template_setup);
 
 static int __init ima_template_fmt_setup(char *str)
 {
-	int num_templates = ARRAY_SIZE(defined_templates);
+	int num_templates = ARRAY_SIZE(builtin_templates);
 
 	if (ima_template)
 		return 1;
@@ -92,22 +98,28 @@ static int __init ima_template_fmt_setup(char *str)
 		return 1;
 	}
 
-	defined_templates[num_templates - 1].fmt = str;
-	ima_template = defined_templates + num_templates - 1;
+	builtin_templates[num_templates - 1].fmt = str;
+	ima_template = builtin_templates + num_templates - 1;
+
 	return 1;
 }
 __setup("ima_template_fmt=", ima_template_fmt_setup);
 
 static struct ima_template_desc *lookup_template_desc(const char *name)
 {
-	int i;
+	struct ima_template_desc *template_desc;
+	int found = 0;
 
-	for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
-		if (strcmp(defined_templates[i].name, name) == 0)
-			return defined_templates + i;
+	rcu_read_lock();
+	list_for_each_entry_rcu(template_desc, &defined_templates, list) {
+		if ((strcmp(template_desc->name, name) == 0) ||
+		    (strcmp(template_desc->fmt, name) == 0)) {
+			found = 1;
+			break;
+		}
 	}
-
-	return NULL;
+	rcu_read_unlock();
+	return found ? template_desc : NULL;
 }
 
 static struct ima_template_field *lookup_template_field(const char *field_id)
@@ -183,11 +195,29 @@ static int template_desc_init_fields(const char *template_fmt,
 	return 0;
 }
 
+void ima_init_template_list(void)
+{
+	int i;
+
+	if (!list_empty(&defined_templates))
+		return;
+
+	spin_lock(&template_list);
+	for (i = 0; i < ARRAY_SIZE(builtin_templates); i++) {
+		list_add_tail_rcu(&builtin_templates[i].list,
+				  &defined_templates);
+	}
+	spin_unlock(&template_list);
+	synchronize_rcu();
+}
+
 struct ima_template_desc *ima_template_desc_current(void)
 {
-	if (!ima_template)
+	if (!ima_template) {
+		ima_init_template_list();
 		ima_template =
 		    lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE);
+	}
 	return ima_template;
 }
 
-- 
2.7.4

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

* [PATCH v6 08/10] ima: support restoring multiple template formats
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (6 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 09/10] ima: define a canonical binary_runtime_measurements list format Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 10/10] ima: platform-independent hash value Thiago Jung Bauermann
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

The configured IMA measurement list template format can be replaced at
runtime on the boot command line, including a custom template format.
This patch adds support for restoring a measuremement list containing
multiple builtin/custom template formats.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_template.c | 53 +++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 3 deletions(-)

diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index c0d808c20c40..e57b4682ff93 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -155,9 +155,14 @@ static int template_desc_init_fields(const char *template_fmt,
 {
 	const char *template_fmt_ptr;
 	struct ima_template_field *found_fields[IMA_TEMPLATE_NUM_FIELDS_MAX];
-	int template_num_fields = template_fmt_size(template_fmt);
+	int template_num_fields;
 	int i, len;
 
+	if (num_fields && *num_fields > 0) /* already initialized? */
+		return 0;
+
+	template_num_fields = template_fmt_size(template_fmt);
+
 	if (template_num_fields > IMA_TEMPLATE_NUM_FIELDS_MAX) {
 		pr_err("format string '%s' contains too many fields\n",
 		       template_fmt);
@@ -237,6 +242,35 @@ int __init ima_init_template(void)
 	return result;
 }
 
+static struct ima_template_desc *restore_template_fmt(char *template_name)
+{
+	struct ima_template_desc *template_desc = NULL;
+	int ret;
+
+	ret = template_desc_init_fields(template_name, NULL, NULL);
+	if (ret < 0) {
+		pr_err("attempting to initialize the template \"%s\" failed\n",
+			template_name);
+		goto out;
+	}
+
+	template_desc = kzalloc(sizeof(*template_desc), GFP_KERNEL);
+	if (!template_desc)
+		goto out;
+
+	template_desc->name = "";
+	template_desc->fmt = kstrdup(template_name, GFP_KERNEL);
+	if (!template_desc->fmt)
+		goto out;
+
+	spin_lock(&template_list);
+	list_add_tail_rcu(&template_desc->list, &defined_templates);
+	spin_unlock(&template_list);
+	synchronize_rcu();
+out:
+	return template_desc;
+}
+
 static int ima_restore_template_data(struct ima_template_desc *template_desc,
 				     void *template_data,
 				     int template_data_size,
@@ -367,10 +401,23 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 		}
 		data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
 
-		/* get template format */
 		template_desc = lookup_template_desc(template_name);
 		if (!template_desc) {
-			pr_err("template \"%s\" not found\n", template_name);
+			template_desc = restore_template_fmt(template_name);
+			if (!template_desc)
+				break;
+		}
+
+		/*
+		 * Only the running system's template format is initialized
+		 * on boot.  As needed, initialize the other template formats.
+		 */
+		ret = template_desc_init_fields(template_desc->fmt,
+						&(template_desc->fields),
+						&(template_desc->num_fields));
+		if (ret < 0) {
+			pr_err("attempting to restore the template fmt \"%s\" \
+				failed\n", template_desc->fmt);
 			ret = -EINVAL;
 			break;
 		}
-- 
2.7.4

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

* [PATCH v6 09/10] ima: define a canonical binary_runtime_measurements list format
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (7 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 08/10] ima: support restoring multiple template formats Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  2016-10-21  2:44 ` [PATCH v6 10/10] ima: platform-independent hash value Thiago Jung Bauermann
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton

From: Mimi Zohar <zohar@linux.vnet.ibm.com>

The IMA binary_runtime_measurements list is currently in platform native
format.

To allow restoring a measurement list carried across kexec with a
different endianness than the targeted kernel, this patch defines
little-endian as the canonical format.  For big endian systems wanting
to save/restore the measurement list from a system with a different
endianness, a new boot command line parameter named "ima_canonical_fmt"
is defined.

Considerations: use of the "ima_canonical_fmt" boot command line
option will break existing userspace applications on big endian systems
expecting the binary_runtime_measurements list to be in platform native
format.

Changelog v3:
- restore PCR value properly

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 Documentation/kernel-parameters.txt       |  4 ++++
 security/integrity/ima/ima.h              |  6 ++++++
 security/integrity/ima/ima_fs.c           | 28 +++++++++++++++++++++-------
 security/integrity/ima/ima_kexec.c        | 11 +++++++++--
 security/integrity/ima/ima_template.c     | 24 ++++++++++++++++++++++--
 security/integrity/ima/ima_template_lib.c |  7 +++++--
 6 files changed, 67 insertions(+), 13 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 37babf91f2cb..3ee81afad7e9 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1641,6 +1641,10 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			The builtin appraise policy appraises all files
 			owned by uid=0.
 
+	ima_canonical_fmt [IMA]
+			Use the canonical format for the binary runtime
+			measurements, instead of host native format.
+
 	ima_hash=	[IMA]
 			Format: { md5 | sha1 | rmd160 | sha256 | sha384
 				   | sha512 | ... }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 6b0540ad189f..5e6180a4da7d 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -122,6 +122,12 @@ void ima_load_kexec_buffer(void);
 static inline void ima_load_kexec_buffer(void) {}
 #endif /* CONFIG_HAVE_IMA_KEXEC */
 
+/*
+ * The default binary_runtime_measurements list format is defined as the
+ * platform native format.  The canonical format is defined as little-endian.
+ */
+extern bool ima_canonical_fmt;
+
 /* Internal IMA function definitions */
 int ima_init(void);
 int ima_fs_init(void);
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 66e5dd5e226f..2bcad99d434e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -28,6 +28,16 @@
 
 static DEFINE_MUTEX(ima_write_mutex);
 
+bool ima_canonical_fmt;
+static int __init default_canonical_fmt_setup(char *str)
+{
+#ifdef __BIG_ENDIAN
+	ima_canonical_fmt = 1;
+#endif
+	return 1;
+}
+__setup("ima_canonical_fmt", default_canonical_fmt_setup);
+
 static int valid_policy = 1;
 #define TMPBUFLEN 12
 static ssize_t ima_show_htable_value(char __user *buf, size_t count,
@@ -122,7 +132,7 @@ int ima_measurements_show(struct seq_file *m, void *v)
 	struct ima_queue_entry *qe = v;
 	struct ima_template_entry *e;
 	char *template_name;
-	int namelen;
+	u32 pcr, namelen, template_data_len; /* temporary fields */
 	bool is_ima_template = false;
 	int i;
 
@@ -139,25 +149,29 @@ int ima_measurements_show(struct seq_file *m, void *v)
 	 * PCR used defaults to the same (config option) in
 	 * little-endian format, unless set in policy
 	 */
-	ima_putc(m, &e->pcr, sizeof(e->pcr));
+	pcr = !ima_canonical_fmt ? e->pcr : cpu_to_le32(e->pcr);
+	ima_putc(m, &pcr, sizeof(e->pcr));
 
 	/* 2nd: template digest */
 	ima_putc(m, e->digest, TPM_DIGEST_SIZE);
 
 	/* 3rd: template name size */
-	namelen = strlen(template_name);
+	namelen = !ima_canonical_fmt ? strlen(template_name) :
+		cpu_to_le32(strlen(template_name));
 	ima_putc(m, &namelen, sizeof(namelen));
 
 	/* 4th:  template name */
-	ima_putc(m, template_name, namelen);
+	ima_putc(m, template_name, strlen(template_name));
 
 	/* 5th:  template length (except for 'ima' template) */
 	if (strcmp(template_name, IMA_TEMPLATE_IMA_NAME) == 0)
 		is_ima_template = true;
 
-	if (!is_ima_template)
-		ima_putc(m, &e->template_data_len,
-			 sizeof(e->template_data_len));
+	if (!is_ima_template) {
+		template_data_len = !ima_canonical_fmt ? e->template_data_len :
+			cpu_to_le32(e->template_data_len);
+		ima_putc(m, &template_data_len, sizeof(e->template_data_len));
+	}
 
 	/* 6th:  template specific data */
 	for (i = 0; i < e->template_desc->num_fields; i++) {
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 2c4824ac1ce1..e473eee913cb 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -21,8 +21,7 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 {
 	struct ima_queue_entry *qe;
 	struct seq_file file;
-	struct ima_kexec_hdr khdr = {
-		.version = 1, .buffer_size = 0, .count = 0};
+	struct ima_kexec_hdr khdr;
 	int ret = 0;
 
 	/* segment size can't change between kexec load and execute */
@@ -36,6 +35,8 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 	file.read_pos = 0;
 	file.count = sizeof(khdr);	/* reserved space */
 
+	memset(&khdr, 0, sizeof(khdr));
+	khdr.version = 1;
 	list_for_each_entry_rcu(qe, &ima_measurements, later) {
 		if (file.count < file.size) {
 			khdr.count++;
@@ -54,7 +55,13 @@ static int ima_dump_measurement_list(unsigned long *buffer_size, void **buffer,
 	 * (eg. version, buffer size, number of measurements)
 	 */
 	khdr.buffer_size = file.count;
+	if (ima_canonical_fmt) {
+		khdr.version = cpu_to_le16(khdr.version);
+		khdr.count = cpu_to_le64(khdr.count);
+		khdr.buffer_size = cpu_to_le64(khdr.buffer_size);
+	}
 	memcpy(file.buf, &khdr, sizeof(khdr));
+
 	print_hex_dump(KERN_DEBUG, "ima dump: ", DUMP_PREFIX_NONE,
 			16, 1, file.buf,
 			file.count < 100 ? file.count : 100, true);
diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
index e57b4682ff93..004723c4169f 100644
--- a/security/integrity/ima/ima_template.c
+++ b/security/integrity/ima/ima_template.c
@@ -304,6 +304,9 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc,
 		}
 		offset += sizeof(field_data->len);
 
+		if (ima_canonical_fmt)
+			field_data->len = le32_to_cpu(field_data->len);
+
 		if (offset > (template_data_size - field_data->len)) {
 			pr_err("Restoring the template field data failed\n");
 			ret = -EINVAL;
@@ -354,7 +357,7 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 	struct binary_data_v1 *data_v1;
 
 	void *bufp = buf + sizeof(*khdr);
-	void *bufendp = buf + khdr->buffer_size;
+	void *bufendp;
 	struct ima_template_entry *entry;
 	struct ima_template_desc *template_desc;
 	unsigned long count = 0;
@@ -363,6 +366,12 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 	if (!buf || size < sizeof(*khdr))
 		return 0;
 
+	if (ima_canonical_fmt) {
+		khdr->version = le16_to_cpu(khdr->version);
+		khdr->count = le64_to_cpu(khdr->count);
+		khdr->buffer_size = le64_to_cpu(khdr->buffer_size);
+	}
+
 	if (khdr->version != 1) {
 		pr_err("attempting to restore a incompatible measurement list");
 		return 0;
@@ -373,6 +382,7 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 	 * v1 format: pcr, digest, template-name-len, template-name,
 	 *	      template-data-size, template-data
 	 */
+	bufendp = buf + khdr->buffer_size;
 	while ((bufp < bufendp) && (count++ < khdr->count)) {
 		if (count > ULONG_MAX - 1) {
 			pr_err("attempting to restore too many measurements");
@@ -380,6 +390,11 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 		}
 
 		hdr_v1 = bufp;
+
+		if (ima_canonical_fmt)
+			hdr_v1->template_name_len =
+			    le32_to_cpu(hdr_v1->template_name_len);
+
 		if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) ||
 		    ((bufp + hdr_v1->template_name_len) > bufendp)) {
 			pr_err("attempting to restore a template name \
@@ -429,6 +444,10 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 		}
 		bufp += (u_int8_t) sizeof(data_v1->template_data_size);
 
+		if (ima_canonical_fmt)
+			data_v1->template_data_size =
+			    le32_to_cpu(data_v1->template_data_size);
+
 		if (bufp > (bufendp - data_v1->template_data_size)) {
 			pr_err("restoring the template data failed\n");
 			ret = -EINVAL;
@@ -443,7 +462,8 @@ int ima_restore_measurement_list(loff_t size, void *buf)
 			break;
 
 		memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
-		entry->pcr = hdr_v1->pcr;
+		entry->pcr =
+		    !ima_canonical_fmt ? hdr_v1->pcr : le32_to_cpu(hdr_v1->pcr);
 		ret = ima_restore_measurement_entry(entry);
 		if (ret < 0)
 			break;
diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c
index f9bae04ba176..f9ba37b3928d 100644
--- a/security/integrity/ima/ima_template_lib.c
+++ b/security/integrity/ima/ima_template_lib.c
@@ -103,8 +103,11 @@ static void ima_show_template_data_binary(struct seq_file *m,
 	u32 len = (show == IMA_SHOW_BINARY_OLD_STRING_FMT) ?
 	    strlen(field_data->data) : field_data->len;
 
-	if (show != IMA_SHOW_BINARY_NO_FIELD_LEN)
-		ima_putc(m, &len, sizeof(len));
+	if (show != IMA_SHOW_BINARY_NO_FIELD_LEN) {
+		u32 field_len = !ima_canonical_fmt ? len : cpu_to_le32(len);
+
+		ima_putc(m, &field_len, sizeof(field_len));
+	}
 
 	if (!len)
 		return;
-- 
2.7.4

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

* [PATCH v6 10/10] ima: platform-independent hash value
  2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
                   ` (8 preceding siblings ...)
  2016-10-21  2:44 ` [PATCH v6 09/10] ima: define a canonical binary_runtime_measurements list format Thiago Jung Bauermann
@ 2016-10-21  2:44 ` Thiago Jung Bauermann
  9 siblings, 0 replies; 18+ messages in thread
From: Thiago Jung Bauermann @ 2016-10-21  2:44 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, Eric W. Biederman, linux-ima-devel, Dave Young,
	kexec, linuxppc-dev, linux-kernel, Andrew Morton,
	Andreas Steffen

From: Andreas Steffen <andreas.steffen@strongswan.org>

For remote attestion it is important for the ima measurement values
to be platform-independent. Therefore integer fields to be hashed
must be converted to canonical format.

Changelog:
- Define canonical format as little endian (Mimi)

Signed-off-by: Andreas Steffen <andreas.steffen@strongswan.org>
Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/integrity/ima/ima_crypto.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 38f2ed830dd6..802d5d20f36f 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -477,11 +477,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
 		u8 buffer[IMA_EVENT_NAME_LEN_MAX + 1] = { 0 };
 		u8 *data_to_hash = field_data[i].data;
 		u32 datalen = field_data[i].len;
+		u32 datalen_to_hash =
+		    !ima_canonical_fmt ? datalen : cpu_to_le32(datalen);
 
 		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
 			rc = crypto_shash_update(shash,
-						(const u8 *) &field_data[i].len,
-						sizeof(field_data[i].len));
+						(const u8 *) &datalen_to_hash,
+						sizeof(datalen_to_hash));
 			if (rc)
 				break;
 		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
-- 
2.7.4

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

* Re: [Linux-ima-devel] [PATCH v6 02/10] ima: on soft reboot, restore the measurement list
  2016-10-21  2:44 ` [PATCH v6 02/10] ima: on soft reboot, restore the measurement list Thiago Jung Bauermann
@ 2016-11-08 19:46   ` Dmitry Kasatkin
  2016-11-08 20:47     ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Kasatkin @ 2016-11-08 19:46 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-security-module, linuxppc-dev, kexec, linux-kernel,
	Eric W. Biederman, linux-ima-devel, Andrew Morton

On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
<bauerman@linux.vnet.ibm.com> wrote:
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> The TPM PCRs are only reset on a hard reboot.  In order to validate a
> TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
> of the running kernel must be saved and restored on boot.  This patch
> restores the measurement list.
>
> Changelog v5:
> - replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
> - replace kexec_get_handover_buffer() with ima_get_kexec_buffer() (Thiago)
> - replace kexec_free_handover_buffer() with ima_free_kexec_buffer() (Thiago)
> - remove unnecessary includes from ima_kexec.c (Thiago)
> - fix off-by-one error when checking hdr_v1->template_name_len (Colin King)
>
> Changelog v2:
> - redefined ima_kexec_hdr to use types with well defined sizes (M. Ellerman)
> - defined missing ima_load_kexec_buffer() stub function
>
> Changelog v1:
> - call ima_load_kexec_buffer() (Thiago)
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/Makefile       |   1 +
>  security/integrity/ima/ima.h          |  21 +++++
>  security/integrity/ima/ima_init.c     |   2 +
>  security/integrity/ima/ima_kexec.c    |  44 +++++++++
>  security/integrity/ima/ima_queue.c    |  10 ++
>  security/integrity/ima/ima_template.c | 170 ++++++++++++++++++++++++++++++++++
>  6 files changed, 248 insertions(+)
>
> diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> index 9aeaedad1e2b..29f198bde02b 100644
> --- a/security/integrity/ima/Makefile
> +++ b/security/integrity/ima/Makefile
> @@ -8,4 +8,5 @@ obj-$(CONFIG_IMA) += ima.o
>  ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
>          ima_policy.o ima_template.o ima_template_lib.o
>  ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> +ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
>  obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index db25f54a04fe..51dc8d57d64d 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -28,6 +28,10 @@
>
>  #include "../integrity.h"
>
> +#ifdef CONFIG_HAVE_IMA_KEXEC
> +#include <asm/ima.h>
> +#endif
> +
>  enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
>                      IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
>  enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> @@ -102,6 +106,21 @@ struct ima_queue_entry {
>  };
>  extern struct list_head ima_measurements;      /* list of all measurements */
>
> +/* Some details preceding the binary serialized measurement list */
> +struct ima_kexec_hdr {
> +       u16 version;
> +       u16 _reserved0;
> +       u32 _reserved1;
> +       u64 buffer_size;
> +       u64 count;
> +};
> +
> +#ifdef CONFIG_HAVE_IMA_KEXEC
> +void ima_load_kexec_buffer(void);
> +#else
> +static inline void ima_load_kexec_buffer(void) {}
> +#endif /* CONFIG_HAVE_IMA_KEXEC */
> +
>  /* Internal IMA function definitions */
>  int ima_init(void);
>  int ima_fs_init(void);
> @@ -122,6 +141,8 @@ int ima_init_crypto(void);
>  void ima_putc(struct seq_file *m, void *data, int datalen);
>  void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
>  struct ima_template_desc *ima_template_desc_current(void);
> +int ima_restore_measurement_entry(struct ima_template_entry *entry);
> +int ima_restore_measurement_list(loff_t bufsize, void *buf);
>  int ima_init_template(void);
>
>  /*
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index 32912bd54ead..3ba0ca49cba6 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -128,6 +128,8 @@ int __init ima_init(void)
>         if (rc != 0)
>                 return rc;
>
> +       ima_load_kexec_buffer();
> +
>         rc = ima_add_boot_aggregate();  /* boot aggregate must be first entry */
>         if (rc != 0)
>                 return rc;
> diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> new file mode 100644
> index 000000000000..36afd0fe9747
> --- /dev/null
> +++ b/security/integrity/ima/ima_kexec.c
> @@ -0,0 +1,44 @@
> +/*
> + * Copyright (C) 2016 IBM Corporation
> + *
> + * Authors:
> + * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> + * Mimi Zohar <zohar@linux.vnet.ibm.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + */
> +#include "ima.h"
> +
> +/*
> + * Restore the measurement list from the previous kernel.
> + */
> +void ima_load_kexec_buffer(void)
> +{
> +       void *kexec_buffer = NULL;
> +       size_t kexec_buffer_size = 0;
> +       int rc;
> +
> +       rc = ima_get_kexec_buffer(&kexec_buffer, &kexec_buffer_size);
> +       switch (rc) {
> +       case 0:
> +               rc = ima_restore_measurement_list(kexec_buffer_size,
> +                                                 kexec_buffer);
> +               if (rc != 0)
> +                       pr_err("Failed to restore the measurement list: %d\n",
> +                               rc);
> +
> +               ima_free_kexec_buffer();
> +               break;
> +       case -ENOTSUPP:
> +               pr_debug("Restoring the measurement list not supported\n");
> +               break;
> +       case -ENOENT:
> +               pr_debug("No measurement list to restore\n");
> +               break;
> +       default:
> +               pr_debug("Error restoring the measurement list: %d\n", rc);
> +       }
> +}
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 32f6ac0f96df..4b1bb7787839 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -149,3 +149,13 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>                             op, audit_cause, result, audit_info);
>         return result;
>  }
> +
> +int ima_restore_measurement_entry(struct ima_template_entry *entry)
> +{
> +       int result = 0;
> +
> +       mutex_lock(&ima_extend_list_mutex);
> +       result = ima_add_digest_entry(entry);
> +       mutex_unlock(&ima_extend_list_mutex);
> +       return result;
> +}
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index febd12ed9b55..37f972cb05fe 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -37,6 +37,7 @@ static struct ima_template_field supported_fields[] = {
>         {.field_id = "sig", .field_init = ima_eventsig_init,
>          .field_show = ima_show_template_sig},
>  };
> +#define MAX_TEMPLATE_NAME_LEN 15
>
>  static struct ima_template_desc *ima_template;
>  static struct ima_template_desc *lookup_template_desc(const char *name);
> @@ -205,3 +206,172 @@ int __init ima_init_template(void)
>
>         return result;
>  }
> +
> +static int ima_restore_template_data(struct ima_template_desc *template_desc,
> +                                    void *template_data,
> +                                    int template_data_size,
> +                                    struct ima_template_entry **entry)
> +{
> +       struct binary_field_data {
> +               u32 len;
> +               u8 data[0];
> +       } __packed;
> +
> +       struct binary_field_data *field_data;
> +       int offset = 0;
> +       int ret = 0;
> +       int i;
> +
> +       *entry = kzalloc(sizeof(**entry) +
> +                   template_desc->num_fields * sizeof(struct ima_field_data),
> +                   GFP_NOFS);
> +       if (!*entry)
> +               return -ENOMEM;
> +
> +       (*entry)->template_desc = template_desc;
> +       for (i = 0; i < template_desc->num_fields; i++) {
> +               field_data = template_data + offset;
> +
> +               /* Each field of the template data is prefixed with a length. */
> +               if (offset > (template_data_size - sizeof(field_data->len))) {
> +                       pr_err("Restoring the template field failed\n");
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +               offset += sizeof(field_data->len);
> +
> +               if (offset > (template_data_size - field_data->len)) {
> +                       pr_err("Restoring the template field data failed\n");
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +               offset += field_data->len;
> +
> +               (*entry)->template_data[i].len = field_data->len;
> +               (*entry)->template_data_len += sizeof(field_data->len);
> +
> +               (*entry)->template_data[i].data =
> +                       kzalloc(field_data->len + 1, GFP_KERNEL);
> +               if (!(*entry)->template_data[i].data) {
> +                       ret = -ENOMEM;
> +                       break;
> +               }
> +               memcpy((*entry)->template_data[i].data, field_data->data,
> +                       field_data->len);
> +               (*entry)->template_data_len += field_data->len;
> +       }
> +
> +       if (ret < 0) {
> +               ima_free_template_entry(*entry);
> +               *entry = NULL;
> +       }
> +
> +       return ret;
> +}
> +
> +/* Restore the serialized binary measurement list without extending PCRs. */
> +int ima_restore_measurement_list(loff_t size, void *buf)
> +{
> +       struct binary_hdr_v1 {
> +               u32 pcr;
> +               u8 digest[TPM_DIGEST_SIZE];
> +               u32 template_name_len;
> +               char template_name[0];
> +       } __packed;
> +       char template_name[MAX_TEMPLATE_NAME_LEN];
> +
> +       struct binary_data_v1 {
> +               u32 template_data_size;
> +               char template_data[0];
> +       } __packed;
> +
> +       struct ima_kexec_hdr *khdr = buf;
> +       struct binary_hdr_v1 *hdr_v1;
> +       struct binary_data_v1 *data_v1;
> +
> +       void *bufp = buf + sizeof(*khdr);
> +       void *bufendp = buf + khdr->buffer_size;
> +       struct ima_template_entry *entry;
> +       struct ima_template_desc *template_desc;
> +       unsigned long count = 0;
> +       int ret = 0;
> +
> +       if (!buf || size < sizeof(*khdr))
> +               return 0;
> +
> +       if (khdr->version != 1) {
> +               pr_err("attempting to restore a incompatible measurement list");
> +               return 0;
> +       }
> +
> +       /*
> +        * ima kexec buffer prefix: version, buffer size, count
> +        * v1 format: pcr, digest, template-name-len, template-name,
> +        *            template-data-size, template-data
> +        */
> +       while ((bufp < bufendp) && (count++ < khdr->count)) {
> +               if (count > ULONG_MAX - 1) {
> +                       pr_err("attempting to restore too many measurements");
> +                       ret = -EINVAL;
> +               }
> +
> +               hdr_v1 = bufp;
> +               if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) ||
> +                   ((bufp + hdr_v1->template_name_len) > bufendp)) {

based on following code  template_name_len does not include header
(sizeof(*hdr_v1))?
If so the check is wrong???


> +                       pr_err("attempting to restore a template name \
> +                               that is too long\n");
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +               bufp += sizeof(*hdr_v1);
> +
> +               /* template name is not null terminated */
> +               memcpy(template_name, bufp, hdr_v1->template_name_len);
> +               template_name[hdr_v1->template_name_len] = 0;
> +
> +               if (strcmp(template_name, "ima") == 0) {
> +                       pr_err("attempting to restore an unsupported \
> +                               template \"%s\" failed\n", template_name);
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +               data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
> +
> +               /* get template format */
> +               template_desc = lookup_template_desc(template_name);
> +               if (!template_desc) {
> +                       pr_err("template \"%s\" not found\n", template_name);
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +
> +               if (bufp > (bufendp - sizeof(data_v1->template_data_size))) {
> +                       pr_err("restoring the template data size failed\n");
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +               bufp += (u_int8_t) sizeof(data_v1->template_data_size);
> +
> +               if (bufp > (bufendp - data_v1->template_data_size)) {
> +                       pr_err("restoring the template data failed\n");
> +                       ret = -EINVAL;
> +                       break;
> +               }
> +

It looks like a similar problem... sizeof(struct binary_data_v1) is
missing in the check...

> +               ret = ima_restore_template_data(template_desc,
> +                                               data_v1->template_data,
> +                                               data_v1->template_data_size,
> +                                               &entry);
> +               if (ret < 0)
> +                       break;
> +
> +               memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
> +               entry->pcr = hdr_v1->pcr;
> +               ret = ima_restore_measurement_entry(entry);
> +               if (ret < 0)
> +                       break;
> +
> +               bufp += data_v1->template_data_size;
> +       }
> +       return ret;
> +}
> --
> 2.7.4
>

In overall it is a bit hard to read this function somehow..

Dmitry

>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



-- 
Thanks,
Dmitry

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

* Re: [Linux-ima-devel] [PATCH v6 03/10] ima: permit duplicate measurement list entries
  2016-10-21  2:44 ` [PATCH v6 03/10] ima: permit duplicate measurement list entries Thiago Jung Bauermann
@ 2016-11-08 19:47   ` Dmitry Kasatkin
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Kasatkin @ 2016-11-08 19:47 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-security-module, linuxppc-dev, kexec, linux-kernel,
	Eric W. Biederman, linux-ima-devel, Andrew Morton

On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
<bauerman@linux.vnet.ibm.com> wrote:
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> Measurements carried across kexec need to be added to the IMA
> measurement list, but should not prevent measurements of the newly
> booted kernel from being added to the measurement list. This patch
> adds support for allowing duplicate measurements.
>
> The "boot_aggregate" measurement entry is the delimiter between soft
> boots.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/ima_queue.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 4b1bb7787839..12d1b040bca9 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -65,11 +65,12 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
>  }
>
>  /* ima_add_template_entry helper function:
> - * - Add template entry to measurement list and hash table.
> + * - Add template entry to the measurement list and hash table, for
> + *   all entries except those carried across kexec.
>   *
>   * (Called with ima_extend_list_mutex held.)
>   */
> -static int ima_add_digest_entry(struct ima_template_entry *entry)
> +static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
>  {
>         struct ima_queue_entry *qe;
>         unsigned int key;
> @@ -85,8 +86,10 @@ static int ima_add_digest_entry(struct ima_template_entry *entry)
>         list_add_tail_rcu(&qe->later, &ima_measurements);
>
>         atomic_long_inc(&ima_htable.len);
> -       key = ima_hash_key(entry->digest);
> -       hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> +       if (flags) {

It looks lile "bool", not flags in fact.


> +               key = ima_hash_key(entry->digest);
> +               hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> +       }
>         return 0;
>  }
>
> @@ -126,7 +129,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>                 }
>         }
>
> -       result = ima_add_digest_entry(entry);
> +       result = ima_add_digest_entry(entry, 1);
>         if (result < 0) {
>                 audit_cause = "ENOMEM";
>                 audit_info = 0;
> @@ -155,7 +158,7 @@ int ima_restore_measurement_entry(struct ima_template_entry *entry)
>         int result = 0;
>
>         mutex_lock(&ima_extend_list_mutex);
> -       result = ima_add_digest_entry(entry);
> +       result = ima_add_digest_entry(entry, 0);
>         mutex_unlock(&ima_extend_list_mutex);
>         return result;
>  }
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



-- 
Thanks,
Dmitry

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

* Re: [Linux-ima-devel] [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list
  2016-10-21  2:44 ` [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list Thiago Jung Bauermann
@ 2016-11-08 20:05   ` Dmitry Kasatkin
  2016-11-08 21:03     ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Dmitry Kasatkin @ 2016-11-08 20:05 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-security-module, linuxppc-dev, kexec, linux-kernel,
	Eric W. Biederman, linux-ima-devel, Andrew Morton

On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
<bauerman@linux.vnet.ibm.com> wrote:
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> In preparation for serializing the binary_runtime_measurements, this patch
> maintains the amount of memory required.
>
> Changelog v5:
> - replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
>
> Changelog v3:
> - include the ima_kexec_hdr size in the binary_runtime_measurement size.
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/Kconfig     | 12 +++++++++
>  security/integrity/ima/ima.h       |  1 +
>  security/integrity/ima/ima_queue.c | 53 ++++++++++++++++++++++++++++++++++++--
>  3 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 5487827fa86c..370eb2f4dd37 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -27,6 +27,18 @@ config IMA
>           to learn more about IMA.
>           If unsure, say N.
>
> +config IMA_KEXEC
> +       bool "Enable carrying the IMA measurement list across a soft boot"
> +       depends on IMA && TCG_TPM && HAVE_IMA_KEXEC
> +       default n
> +       help
> +          TPM PCRs are only reset on a hard reboot.  In order to validate
> +          a TPM's quote after a soft boot, the IMA measurement list of the
> +          running kernel must be saved and restored on boot.
> +
> +          Depending on the IMA policy, the measurement list can grow to
> +          be very large.
> +
>  config IMA_MEASURE_PCR_IDX
>         int
>         depends on IMA
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 51dc8d57d64d..ea1dcc452911 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -143,6 +143,7 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
>  struct ima_template_desc *ima_template_desc_current(void);
>  int ima_restore_measurement_entry(struct ima_template_entry *entry);
>  int ima_restore_measurement_list(loff_t bufsize, void *buf);
> +unsigned long ima_get_binary_runtime_size(void);
>  int ima_init_template(void);
>
>  /*
> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> index 12d1b040bca9..3a3cc2a45645 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -29,6 +29,11 @@
>  #define AUDIT_CAUSE_LEN_MAX 32
>
>  LIST_HEAD(ima_measurements);   /* list of all measurements */
> +#ifdef CONFIG_IMA_KEXEC
> +static unsigned long binary_runtime_size;
> +#else
> +static unsigned long binary_runtime_size = ULONG_MAX;
> +#endif
>
>  /* key: inode (before secure-hashing a file) */
>  struct ima_h_table ima_htable = {
> @@ -64,6 +69,24 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
>         return ret;
>  }
>
> +/*
> + * Calculate the memory required for serializing a single
> + * binary_runtime_measurement list entry, which contains a
> + * couple of variable length fields (e.g template name and data).
> + */
> +static int get_binary_runtime_size(struct ima_template_entry *entry)
> +{
> +       int size = 0;
> +
> +       size += sizeof(u32);    /* pcr */
> +       size += sizeof(entry->digest);
> +       size += sizeof(int);    /* template name size field */
> +       size += strlen(entry->template_desc->name);
> +       size += sizeof(entry->template_data_len);
> +       size += entry->template_data_len;
> +       return size;
> +}
> +

strlen returns len without '\0'. I cannot see how you would know how
to read it back?


>  /* ima_add_template_entry helper function:
>   * - Add template entry to the measurement list and hash table, for
>   *   all entries except those carried across kexec.
> @@ -90,9 +113,30 @@ static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
>                 key = ima_hash_key(entry->digest);
>                 hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
>         }
> +
> +       if (binary_runtime_size != ULONG_MAX) {
> +               int size;
> +
> +               size = get_binary_runtime_size(entry);
> +               binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
> +                    binary_runtime_size + size : ULONG_MAX;
> +       }
>         return 0;
>  }
>
> +/*
> + * Return the amount of memory required for serializing the
> + * entire binary_runtime_measurement list, including the ima_kexec_hdr
> + * structure.
> + */
> +unsigned long ima_get_binary_runtime_size(void)
> +{
> +       if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> +               return ULONG_MAX;
> +       else
> +               return binary_runtime_size + sizeof(struct ima_kexec_hdr);
> +};
> +
>  static int ima_pcr_extend(const u8 *hash, int pcr)
>  {
>         int result = 0;
> @@ -106,8 +150,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
>         return result;
>  }
>
> -/* Add template entry to the measurement list and hash table,
> - * and extend the pcr.
> +/*
> + * Add template entry to the measurement list and hash table, and
> + * extend the pcr.
> + *
> + * On systems which support carrying the IMA measurement list across
> + * kexec, maintain the total memory size required for serializing the
> + * binary_runtime_measurements.
>   */
>  int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>                            const char *op, struct inode *inode,
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



-- 
Thanks,
Dmitry

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

* Re: [Linux-ima-devel] [PATCH v6 02/10] ima: on soft reboot, restore the measurement list
  2016-11-08 19:46   ` [Linux-ima-devel] " Dmitry Kasatkin
@ 2016-11-08 20:47     ` Mimi Zohar
  2016-11-10 13:12       ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2016-11-08 20:47 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: Thiago Jung Bauermann, linux-security-module, linuxppc-dev,
	kexec, linux-kernel, Eric W. Biederman, linux-ima-devel,
	Andrew Morton

On Tue, 2016-11-08 at 21:46 +0200, Dmitry Kasatkin wrote:
> On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
> <bauerman@linux.vnet.ibm.com> wrote:
> > From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > The TPM PCRs are only reset on a hard reboot.  In order to validate a
> > TPM's quote after a soft reboot (eg. kexec -e), the IMA measurement list
> > of the running kernel must be saved and restored on boot.  This patch
> > restores the measurement list.
> >
> > Changelog v5:
> > - replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
> > - replace kexec_get_handover_buffer() with ima_get_kexec_buffer() (Thiago)
> > - replace kexec_free_handover_buffer() with ima_free_kexec_buffer() (Thiago)
> > - remove unnecessary includes from ima_kexec.c (Thiago)
> > - fix off-by-one error when checking hdr_v1->template_name_len (Colin King)
> >
> > Changelog v2:
> > - redefined ima_kexec_hdr to use types with well defined sizes (M. Ellerman)
> > - defined missing ima_load_kexec_buffer() stub function
> >
> > Changelog v1:
> > - call ima_load_kexec_buffer() (Thiago)
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > ---
> >  security/integrity/ima/Makefile       |   1 +
> >  security/integrity/ima/ima.h          |  21 +++++
> >  security/integrity/ima/ima_init.c     |   2 +
> >  security/integrity/ima/ima_kexec.c    |  44 +++++++++
> >  security/integrity/ima/ima_queue.c    |  10 ++
> >  security/integrity/ima/ima_template.c | 170 ++++++++++++++++++++++++++++++++++
> >  6 files changed, 248 insertions(+)
> >
> > diff --git a/security/integrity/ima/Makefile b/security/integrity/ima/Makefile
> > index 9aeaedad1e2b..29f198bde02b 100644
> > --- a/security/integrity/ima/Makefile
> > +++ b/security/integrity/ima/Makefile
> > @@ -8,4 +8,5 @@ obj-$(CONFIG_IMA) += ima.o
> >  ima-y := ima_fs.o ima_queue.o ima_init.o ima_main.o ima_crypto.o ima_api.o \
> >          ima_policy.o ima_template.o ima_template_lib.o
> >  ima-$(CONFIG_IMA_APPRAISE) += ima_appraise.o
> > +ima-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o
> >  obj-$(CONFIG_IMA_BLACKLIST_KEYRING) += ima_mok.o
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index db25f54a04fe..51dc8d57d64d 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -28,6 +28,10 @@
> >
> >  #include "../integrity.h"
> >
> > +#ifdef CONFIG_HAVE_IMA_KEXEC
> > +#include <asm/ima.h>
> > +#endif
> > +
> >  enum ima_show_type { IMA_SHOW_BINARY, IMA_SHOW_BINARY_NO_FIELD_LEN,
> >                      IMA_SHOW_BINARY_OLD_STRING_FMT, IMA_SHOW_ASCII };
> >  enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 };
> > @@ -102,6 +106,21 @@ struct ima_queue_entry {
> >  };
> >  extern struct list_head ima_measurements;      /* list of all measurements */
> >
> > +/* Some details preceding the binary serialized measurement list */
> > +struct ima_kexec_hdr {
> > +       u16 version;
> > +       u16 _reserved0;
> > +       u32 _reserved1;
> > +       u64 buffer_size;
> > +       u64 count;
> > +};
> > +
> > +#ifdef CONFIG_HAVE_IMA_KEXEC
> > +void ima_load_kexec_buffer(void);
> > +#else
> > +static inline void ima_load_kexec_buffer(void) {}
> > +#endif /* CONFIG_HAVE_IMA_KEXEC */
> > +
> >  /* Internal IMA function definitions */
> >  int ima_init(void);
> >  int ima_fs_init(void);
> > @@ -122,6 +141,8 @@ int ima_init_crypto(void);
> >  void ima_putc(struct seq_file *m, void *data, int datalen);
> >  void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
> >  struct ima_template_desc *ima_template_desc_current(void);
> > +int ima_restore_measurement_entry(struct ima_template_entry *entry);
> > +int ima_restore_measurement_list(loff_t bufsize, void *buf);
> >  int ima_init_template(void);
> >
> >  /*
> > diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> > index 32912bd54ead..3ba0ca49cba6 100644
> > --- a/security/integrity/ima/ima_init.c
> > +++ b/security/integrity/ima/ima_init.c
> > @@ -128,6 +128,8 @@ int __init ima_init(void)
> >         if (rc != 0)
> >                 return rc;
> >
> > +       ima_load_kexec_buffer();
> > +
> >         rc = ima_add_boot_aggregate();  /* boot aggregate must be first entry */
> >         if (rc != 0)
> >                 return rc;
> > diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
> > new file mode 100644
> > index 000000000000..36afd0fe9747
> > --- /dev/null
> > +++ b/security/integrity/ima/ima_kexec.c
> > @@ -0,0 +1,44 @@
> > +/*
> > + * Copyright (C) 2016 IBM Corporation
> > + *
> > + * Authors:
> > + * Thiago Jung Bauermann <bauerman@linux.vnet.ibm.com>
> > + * Mimi Zohar <zohar@linux.vnet.ibm.com>
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + */
> > +#include "ima.h"
> > +
> > +/*
> > + * Restore the measurement list from the previous kernel.
> > + */
> > +void ima_load_kexec_buffer(void)
> > +{
> > +       void *kexec_buffer = NULL;
> > +       size_t kexec_buffer_size = 0;
> > +       int rc;
> > +
> > +       rc = ima_get_kexec_buffer(&kexec_buffer, &kexec_buffer_size);
> > +       switch (rc) {
> > +       case 0:
> > +               rc = ima_restore_measurement_list(kexec_buffer_size,
> > +                                                 kexec_buffer);
> > +               if (rc != 0)
> > +                       pr_err("Failed to restore the measurement list: %d\n",
> > +                               rc);
> > +
> > +               ima_free_kexec_buffer();
> > +               break;
> > +       case -ENOTSUPP:
> > +               pr_debug("Restoring the measurement list not supported\n");
> > +               break;
> > +       case -ENOENT:
> > +               pr_debug("No measurement list to restore\n");
> > +               break;
> > +       default:
> > +               pr_debug("Error restoring the measurement list: %d\n", rc);
> > +       }
> > +}
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index 32f6ac0f96df..4b1bb7787839 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -149,3 +149,13 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> >                             op, audit_cause, result, audit_info);
> >         return result;
> >  }
> > +
> > +int ima_restore_measurement_entry(struct ima_template_entry *entry)
> > +{
> > +       int result = 0;
> > +
> > +       mutex_lock(&ima_extend_list_mutex);
> > +       result = ima_add_digest_entry(entry);
> > +       mutex_unlock(&ima_extend_list_mutex);
> > +       return result;
> > +}
> > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> > index febd12ed9b55..37f972cb05fe 100644
> > --- a/security/integrity/ima/ima_template.c
> > +++ b/security/integrity/ima/ima_template.c
> > @@ -37,6 +37,7 @@ static struct ima_template_field supported_fields[] = {
> >         {.field_id = "sig", .field_init = ima_eventsig_init,
> >          .field_show = ima_show_template_sig},
> >  };
> > +#define MAX_TEMPLATE_NAME_LEN 15
> >
> >  static struct ima_template_desc *ima_template;
> >  static struct ima_template_desc *lookup_template_desc(const char *name);
> > @@ -205,3 +206,172 @@ int __init ima_init_template(void)
> >
> >         return result;
> >  }
> > +
> > +static int ima_restore_template_data(struct ima_template_desc *template_desc,
> > +                                    void *template_data,
> > +                                    int template_data_size,
> > +                                    struct ima_template_entry **entry)
> > +{
> > +       struct binary_field_data {
> > +               u32 len;
> > +               u8 data[0];
> > +       } __packed;
> > +
> > +       struct binary_field_data *field_data;
> > +       int offset = 0;
> > +       int ret = 0;
> > +       int i;
> > +
> > +       *entry = kzalloc(sizeof(**entry) +
> > +                   template_desc->num_fields * sizeof(struct ima_field_data),
> > +                   GFP_NOFS);
> > +       if (!*entry)
> > +               return -ENOMEM;
> > +
> > +       (*entry)->template_desc = template_desc;
> > +       for (i = 0; i < template_desc->num_fields; i++) {
> > +               field_data = template_data + offset;
> > +
> > +               /* Each field of the template data is prefixed with a length. */
> > +               if (offset > (template_data_size - sizeof(field_data->len))) {
> > +                       pr_err("Restoring the template field failed\n");
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +               offset += sizeof(field_data->len);
> > +
> > +               if (offset > (template_data_size - field_data->len)) {
> > +                       pr_err("Restoring the template field data failed\n");
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +               offset += field_data->len;
> > +
> > +               (*entry)->template_data[i].len = field_data->len;
> > +               (*entry)->template_data_len += sizeof(field_data->len);
> > +
> > +               (*entry)->template_data[i].data =
> > +                       kzalloc(field_data->len + 1, GFP_KERNEL);
> > +               if (!(*entry)->template_data[i].data) {
> > +                       ret = -ENOMEM;
> > +                       break;
> > +               }
> > +               memcpy((*entry)->template_data[i].data, field_data->data,
> > +                       field_data->len);
> > +               (*entry)->template_data_len += field_data->len;
> > +       }
> > +
> > +       if (ret < 0) {
> > +               ima_free_template_entry(*entry);
> > +               *entry = NULL;
> > +       }
> > +
> > +       return ret;
> > +}
> > +
> > +/* Restore the serialized binary measurement list without extending PCRs. */
> > +int ima_restore_measurement_list(loff_t size, void *buf)
> > +{
> > +       struct binary_hdr_v1 {
> > +               u32 pcr;
> > +               u8 digest[TPM_DIGEST_SIZE];
> > +               u32 template_name_len;
> > +               char template_name[0];
> > +       } __packed;
> > +       char template_name[MAX_TEMPLATE_NAME_LEN];
> > +
> > +       struct binary_data_v1 {
> > +               u32 template_data_size;
> > +               char template_data[0];
> > +       } __packed;
> > +
> > +       struct ima_kexec_hdr *khdr = buf;
> > +       struct binary_hdr_v1 *hdr_v1;
> > +       struct binary_data_v1 *data_v1;
> > +
> > +       void *bufp = buf + sizeof(*khdr);
> > +       void *bufendp = buf + khdr->buffer_size;
> > +       struct ima_template_entry *entry;
> > +       struct ima_template_desc *template_desc;
> > +       unsigned long count = 0;
> > +       int ret = 0;
> > +
> > +       if (!buf || size < sizeof(*khdr))
> > +               return 0;
> > +
> > +       if (khdr->version != 1) {
> > +               pr_err("attempting to restore a incompatible measurement list");
> > +               return 0;
> > +       }
> > +
> > +       /*
> > +        * ima kexec buffer prefix: version, buffer size, count
> > +        * v1 format: pcr, digest, template-name-len, template-name,
> > +        *            template-data-size, template-data
> > +        */
> > +       while ((bufp < bufendp) && (count++ < khdr->count)) {
> > +               if (count > ULONG_MAX - 1) {
> > +                       pr_err("attempting to restore too many measurements");
> > +                       ret = -EINVAL;
> > +               }
> > +
> > +               hdr_v1 = bufp;
> > +               if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) ||
> > +                   ((bufp + hdr_v1->template_name_len) > bufendp)) {
> 
> based on following code  template_name_len does not include header
> (sizeof(*hdr_v1))?
> If so the check is wrong???

Yes, good catch.  In addition, before assigning hdr_v1 there should be a
size check as well. 

> 
> > +                       pr_err("attempting to restore a template name \
> > +                               that is too long\n");
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +               bufp += sizeof(*hdr_v1);
> > +
> > +               /* template name is not null terminated */
> > +               memcpy(template_name, bufp, hdr_v1->template_name_len);
> > +               template_name[hdr_v1->template_name_len] = 0;
> > +
> > +               if (strcmp(template_name, "ima") == 0) {
> > +                       pr_err("attempting to restore an unsupported \
> > +                               template \"%s\" failed\n", template_name);
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +               data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
> > +
> > +               /* get template format */
> > +               template_desc = lookup_template_desc(template_name);
> > +               if (!template_desc) {
> > +                       pr_err("template \"%s\" not found\n", template_name);
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +
> > +               if (bufp > (bufendp - sizeof(data_v1->template_data_size))) {
> > +                       pr_err("restoring the template data size failed\n");
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +               bufp += (u_int8_t) sizeof(data_v1->template_data_size);
> > +
> > +               if (bufp > (bufendp - data_v1->template_data_size)) {
> > +                       pr_err("restoring the template data failed\n");
> > +                       ret = -EINVAL;
> > +                       break;
> > +               }
> > +
> 
> It looks like a similar problem... sizeof(struct binary_data_v1) is
> missing in the check...

Following the template name, is the template data length and the
template data. 

> > +               ret = ima_restore_template_data(template_desc,
> > +                                               data_v1->template_data,
> > +                                               data_v1->template_data_size,
> > +                                               &entry);
> > +               if (ret < 0)
> > +                       break;
> > +
> > +               memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
> > +               entry->pcr = hdr_v1->pcr;
> > +               ret = ima_restore_measurement_entry(entry);
> > +               if (ret < 0)
> > +                       break;
> > +
> > +               bufp += data_v1->template_data_size;
> > +       }
> > +       return ret;
> > +}
> > --
> > 2.7.4
> >
> 
> In overall it is a bit hard to read this function somehow..

Ok, I'll see if there is any way of simplifying/cleaning up walking the
measurement list some more.

Mimi

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

* Re: [Linux-ima-devel] [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list
  2016-11-08 20:05   ` [Linux-ima-devel] " Dmitry Kasatkin
@ 2016-11-08 21:03     ` Mimi Zohar
  0 siblings, 0 replies; 18+ messages in thread
From: Mimi Zohar @ 2016-11-08 21:03 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: Thiago Jung Bauermann, linux-security-module, linuxppc-dev,
	kexec, linux-kernel, Eric W. Biederman, linux-ima-devel,
	Andrew Morton

On Tue, 2016-11-08 at 22:05 +0200, Dmitry Kasatkin wrote:
> On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
> <bauerman@linux.vnet.ibm.com> wrote:
> > From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >
> > In preparation for serializing the binary_runtime_measurements, this patch
> > maintains the amount of memory required.
> >
> > Changelog v5:
> > - replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
> >
> > Changelog v3:
> > - include the ima_kexec_hdr size in the binary_runtime_measurement size.
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > ---
> >  security/integrity/ima/Kconfig     | 12 +++++++++
> >  security/integrity/ima/ima.h       |  1 +
> >  security/integrity/ima/ima_queue.c | 53 ++++++++++++++++++++++++++++++++++++--
> >  3 files changed, 64 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 5487827fa86c..370eb2f4dd37 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -27,6 +27,18 @@ config IMA
> >           to learn more about IMA.
> >           If unsure, say N.
> >
> > +config IMA_KEXEC
> > +       bool "Enable carrying the IMA measurement list across a soft boot"
> > +       depends on IMA && TCG_TPM && HAVE_IMA_KEXEC
> > +       default n
> > +       help
> > +          TPM PCRs are only reset on a hard reboot.  In order to validate
> > +          a TPM's quote after a soft boot, the IMA measurement list of the
> > +          running kernel must be saved and restored on boot.
> > +
> > +          Depending on the IMA policy, the measurement list can grow to
> > +          be very large.
> > +
> >  config IMA_MEASURE_PCR_IDX
> >         int
> >         depends on IMA
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 51dc8d57d64d..ea1dcc452911 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -143,6 +143,7 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32 size);
> >  struct ima_template_desc *ima_template_desc_current(void);
> >  int ima_restore_measurement_entry(struct ima_template_entry *entry);
> >  int ima_restore_measurement_list(loff_t bufsize, void *buf);
> > +unsigned long ima_get_binary_runtime_size(void);
> >  int ima_init_template(void);
> >
> >  /*
> > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c
> > index 12d1b040bca9..3a3cc2a45645 100644
> > --- a/security/integrity/ima/ima_queue.c
> > +++ b/security/integrity/ima/ima_queue.c
> > @@ -29,6 +29,11 @@
> >  #define AUDIT_CAUSE_LEN_MAX 32
> >
> >  LIST_HEAD(ima_measurements);   /* list of all measurements */
> > +#ifdef CONFIG_IMA_KEXEC
> > +static unsigned long binary_runtime_size;
> > +#else
> > +static unsigned long binary_runtime_size = ULONG_MAX;
> > +#endif
> >
> >  /* key: inode (before secure-hashing a file) */
> >  struct ima_h_table ima_htable = {
> > @@ -64,6 +69,24 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8 *digest_value,
> >         return ret;
> >  }
> >
> > +/*
> > + * Calculate the memory required for serializing a single
> > + * binary_runtime_measurement list entry, which contains a
> > + * couple of variable length fields (e.g template name and data).
> > + */
> > +static int get_binary_runtime_size(struct ima_template_entry *entry)
> > +{
> > +       int size = 0;
> > +
> > +       size += sizeof(u32);    /* pcr */
> > +       size += sizeof(entry->digest);
> > +       size += sizeof(int);    /* template name size field */
> > +       size += strlen(entry->template_desc->name);
> > +       size += sizeof(entry->template_data_len);
> > +       size += entry->template_data_len;
> > +       return size;
> > +}
> > +
> 
> strlen returns len without '\0'. I cannot see how you would know how
> to read it back?

This function is used to keep a running memory size needed for
allocating the buffer to carry the measurement list across kexec.

Right, the memory need for the template name is +1.

Mimi

> 
> >  /* ima_add_template_entry helper function:
> >   * - Add template entry to the measurement list and hash table, for
> >   *   all entries except those carried across kexec.
> > @@ -90,9 +113,30 @@ static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
> >                 key = ima_hash_key(entry->digest);
> >                 hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> >         }
> > +
> > +       if (binary_runtime_size != ULONG_MAX) {
> > +               int size;
> > +
> > +               size = get_binary_runtime_size(entry);
> > +               binary_runtime_size = (binary_runtime_size < ULONG_MAX - size) ?
> > +                    binary_runtime_size + size : ULONG_MAX;
> > +       }
> >         return 0;
> >  }
> >
> > +/*
> > + * Return the amount of memory required for serializing the
> > + * entire binary_runtime_measurement list, including the ima_kexec_hdr
> > + * structure.
> > + */
> > +unsigned long ima_get_binary_runtime_size(void)
> > +{
> > +       if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> > +               return ULONG_MAX;
> > +       else
> > +               return binary_runtime_size + sizeof(struct ima_kexec_hdr);
> > +};
> > +
> >  static int ima_pcr_extend(const u8 *hash, int pcr)
> >  {
> >         int result = 0;
> > @@ -106,8 +150,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
> >         return result;
> >  }
> >
> > -/* Add template entry to the measurement list and hash table,
> > - * and extend the pcr.
> > +/*
> > + * Add template entry to the measurement list and hash table, and
> > + * extend the pcr.
> > + *
> > + * On systems which support carrying the IMA measurement list across
> > + * kexec, maintain the total memory size required for serializing the
> > + * binary_runtime_measurements.
> >   */
> >  int ima_add_template_entry(struct ima_template_entry *entry, int violation,
> >                            const char *op, struct inode *inode,
> > --
> > 2.7.4

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

* Re: [Linux-ima-devel] [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list
  2016-10-21  2:44 ` [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list Thiago Jung Bauermann
@ 2016-11-08 23:40   ` Dmitry Kasatkin
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Kasatkin @ 2016-11-08 23:40 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: linux-security-module, linuxppc-dev, kexec, linux-kernel,
	Eric W. Biederman, linux-ima-devel, Andrew Morton

On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
<bauerman@linux.vnet.ibm.com> wrote:
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>
> The builtin and single custom templates are currently stored in an
> array.  In preparation for being able to restore a measurement list
> containing multiple builtin/custom templates, this patch stores the
> builtin and custom templates as a linked list.  This will permit
> defining more than one custom template per boot.
>
> Changelog v4:
> - fix "spinlock bad magic" BUG - reported by Dmitry Vyukov
>
> Changelog v3:
> - initialize template format list in ima_template_desc_current(), as it
> might be called during __setup before normal initialization. (kernel
> test robot)
> - remove __init annotation of ima_init_template_list()
>
> Changelog v2:
> - fix lookup_template_desc() preemption imbalance (kernel test robot)
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  security/integrity/ima/ima.h          |  2 ++
>  security/integrity/ima/ima_main.c     |  1 +
>  security/integrity/ima/ima_template.c | 52 +++++++++++++++++++++++++++--------
>  3 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 139dec67dcbf..6b0540ad189f 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -85,6 +85,7 @@ struct ima_template_field {
>
>  /* IMA template descriptor definition */
>  struct ima_template_desc {
> +       struct list_head list;
>         char *name;
>         char *fmt;
>         int num_fields;
> @@ -146,6 +147,7 @@ int ima_restore_measurement_list(loff_t bufsize, void *buf);
>  int ima_measurements_show(struct seq_file *m, void *v);
>  unsigned long ima_get_binary_runtime_size(void);
>  int ima_init_template(void);
> +void ima_init_template_list(void);
>
>  /*
>   * used to protect h_table and sha_table
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 423d111b3b94..50818c60538b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -418,6 +418,7 @@ static int __init init_ima(void)
>  {
>         int error;
>
> +       ima_init_template_list();
>         hash_setup(CONFIG_IMA_DEFAULT_HASH);
>         error = ima_init();
>         if (!error) {
> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c
> index 37f972cb05fe..c0d808c20c40 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -15,16 +15,20 @@
>
>  #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include <linux/rculist.h>
>  #include "ima.h"
>  #include "ima_template_lib.h"
>
> -static struct ima_template_desc defined_templates[] = {
> +static struct ima_template_desc builtin_templates[] = {
>         {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT},
>         {.name = "ima-ng", .fmt = "d-ng|n-ng"},
>         {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
>         {.name = "", .fmt = ""},        /* placeholder for a custom format */
>  };
>
> +static LIST_HEAD(defined_templates);
> +static DEFINE_SPINLOCK(template_list);
> +
>  static struct ima_template_field supported_fields[] = {
>         {.field_id = "d", .field_init = ima_eventdigest_init,
>          .field_show = ima_show_template_digest},
> @@ -53,6 +57,8 @@ static int __init ima_template_setup(char *str)
>         if (ima_template)
>                 return 1;
>
> +       ima_init_template_list();
> +
>         /*
>          * Verify that a template with the supplied name exists.
>          * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
> @@ -81,7 +87,7 @@ __setup("ima_template=", ima_template_setup);
>
>  static int __init ima_template_fmt_setup(char *str)
>  {
> -       int num_templates = ARRAY_SIZE(defined_templates);
> +       int num_templates = ARRAY_SIZE(builtin_templates);
>
>         if (ima_template)
>                 return 1;
> @@ -92,22 +98,28 @@ static int __init ima_template_fmt_setup(char *str)
>                 return 1;
>         }
>
> -       defined_templates[num_templates - 1].fmt = str;
> -       ima_template = defined_templates + num_templates - 1;
> +       builtin_templates[num_templates - 1].fmt = str;
> +       ima_template = builtin_templates + num_templates - 1;
> +
>         return 1;
>  }
>  __setup("ima_template_fmt=", ima_template_fmt_setup);
>
>  static struct ima_template_desc *lookup_template_desc(const char *name)
>  {
> -       int i;
> +       struct ima_template_desc *template_desc;
> +       int found = 0;
>
> -       for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
> -               if (strcmp(defined_templates[i].name, name) == 0)
> -                       return defined_templates + i;
> +       rcu_read_lock();
> +       list_for_each_entry_rcu(template_desc, &defined_templates, list) {
> +               if ((strcmp(template_desc->name, name) == 0) ||
> +                   (strcmp(template_desc->fmt, name) == 0)) {
> +                       found = 1;
> +                       break;
> +               }
>         }
> -
> -       return NULL;
> +       rcu_read_unlock();
> +       return found ? template_desc : NULL;
>  }
>
>  static struct ima_template_field *lookup_template_field(const char *field_id)
> @@ -183,11 +195,29 @@ static int template_desc_init_fields(const char *template_fmt,
>         return 0;
>  }
>
> +void ima_init_template_list(void)
> +{
> +       int i;
> +
> +       if (!list_empty(&defined_templates))
> +               return;
> +
> +       spin_lock(&template_list);
> +       for (i = 0; i < ARRAY_SIZE(builtin_templates); i++) {
> +               list_add_tail_rcu(&builtin_templates[i].list,
> +                                 &defined_templates);
> +       }
> +       spin_unlock(&template_list);
> +       synchronize_rcu();

Btw, what is the purpose of synchronize rcu here?
i think it make sense before deleting the object, like:
--------------------------------
list_del_rcu(&p->list);
spin_unlock(&listmutex);
synchronize_rcu();
kfree(p);
--------------------------------

I think it is never deleted.
There is another similar place in the same file...


> +}
> +
>  struct ima_template_desc *ima_template_desc_current(void)
>  {
> -       if (!ima_template)
> +       if (!ima_template) {
> +               ima_init_template_list();
>                 ima_template =
>                     lookup_template_desc(CONFIG_IMA_DEFAULT_TEMPLATE);
> +       }
>         return ima_template;
>  }
>
> --
> 2.7.4
>
>
> ------------------------------------------------------------------------------
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> _______________________________________________
> Linux-ima-devel mailing list
> Linux-ima-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel



-- 
Thanks,
Dmitry

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

* Re: [Linux-ima-devel] [PATCH v6 02/10] ima: on soft reboot, restore the measurement list
  2016-11-08 20:47     ` Mimi Zohar
@ 2016-11-10 13:12       ` Mimi Zohar
  0 siblings, 0 replies; 18+ messages in thread
From: Mimi Zohar @ 2016-11-10 13:12 UTC (permalink / raw)
  To: Dmitry Kasatkin
  Cc: kexec, linux-kernel, linux-ima-devel, linux-security-module,
	Eric W. Biederman, Thiago Jung Bauermann, Andrew Morton,
	linuxppc-dev

On Tue, 2016-11-08 at 15:47 -0500, Mimi Zohar wrote:
> On Tue, 2016-11-08 at 21:46 +0200, Dmitry Kasatkin wrote:
> > On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann

> > > +/* Restore the serialized binary measurement list without extending PCRs. */
> > > +int ima_restore_measurement_list(loff_t size, void *buf)
> > > +{
> > > +       struct binary_hdr_v1 {
> > > +               u32 pcr;
> > > +               u8 digest[TPM_DIGEST_SIZE];
> > > +               u32 template_name_len;
> > > +               char template_name[0];
> > > +       } __packed;
> > > +       char template_name[MAX_TEMPLATE_NAME_LEN];
> > > +
> > > +       struct binary_data_v1 {
> > > +               u32 template_data_size;
> > > +               char template_data[0];
> > > +       } __packed;
> > > +
> > > +       struct ima_kexec_hdr *khdr = buf;
> > > +       struct binary_hdr_v1 *hdr_v1;
> > > +       struct binary_data_v1 *data_v1;
> > > +
> > > +       void *bufp = buf + sizeof(*khdr);
> > > +       void *bufendp = buf + khdr->buffer_size;
> > > +       struct ima_template_entry *entry;
> > > +       struct ima_template_desc *template_desc;
> > > +       unsigned long count = 0;
> > > +       int ret = 0;
> > > +
> > > +       if (!buf || size < sizeof(*khdr))
> > > +               return 0;
> > > +
> > > +       if (khdr->version != 1) {
> > > +               pr_err("attempting to restore a incompatible measurement list");
> > > +               return 0;
> > > +       }
> > > +
> > > +       /*
> > > +        * ima kexec buffer prefix: version, buffer size, count
> > > +        * v1 format: pcr, digest, template-name-len, template-name,
> > > +        *            template-data-size, template-data
> > > +        */
> > > +       while ((bufp < bufendp) && (count++ < khdr->count)) {
> > > +               if (count > ULONG_MAX - 1) {
> > > +                       pr_err("attempting to restore too many measurements");
> > > +                       ret = -EINVAL;
> > > +               }
> > > +
> > > +               hdr_v1 = bufp;
> > > +               if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) ||
> > > +                   ((bufp + hdr_v1->template_name_len) > bufendp)) {
> > 
> > based on following code  template_name_len does not include header
> > (sizeof(*hdr_v1))?
> > If so the check is wrong???
> 
> Yes, good catch.  In addition, before assigning hdr_v1 there should be a
> size check as well. 
> 
> > 
> > > +                       pr_err("attempting to restore a template name \
> > > +                               that is too long\n");
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +               bufp += sizeof(*hdr_v1);

This line should have been before the test above.

> > > +
> > > +               /* template name is not null terminated */
> > > +               memcpy(template_name, bufp, hdr_v1->template_name_len);
> > > +               template_name[hdr_v1->template_name_len] = 0;
> > > +
> > > +               if (strcmp(template_name, "ima") == 0) {
> > > +                       pr_err("attempting to restore an unsupported \
> > > +                               template \"%s\" failed\n", template_name);
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +               data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len;
> > > +
> > > +               /* get template format */
> > > +               template_desc = lookup_template_desc(template_name);
> > > +               if (!template_desc) {
> > > +                       pr_err("template \"%s\" not found\n", template_name);
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +
> > > +               if (bufp > (bufendp - sizeof(data_v1->template_data_size))) {
> > > +                       pr_err("restoring the template data size failed\n");
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +               bufp += (u_int8_t) sizeof(data_v1->template_data_size);
> > > +
> > > +               if (bufp > (bufendp - data_v1->template_data_size)) {
> > > +                       pr_err("restoring the template data failed\n");
> > > +                       ret = -EINVAL;
> > > +                       break;
> > > +               }
> > > +
> > 
> > It looks like a similar problem... sizeof(struct binary_data_v1) is
> > missing in the check...
> 
> Following the template name, is the template data length and the
> template data. 
> 
> > > +               ret = ima_restore_template_data(template_desc,
> > > +                                               data_v1->template_data,
> > > +                                               data_v1->template_data_size,
> > > +                                               &entry);
> > > +               if (ret < 0)
> > > +                       break;
> > > +
> > > +               memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE);
> > > +               entry->pcr = hdr_v1->pcr;
> > > +               ret = ima_restore_measurement_entry(entry);
> > > +               if (ret < 0)
> > > +                       break;
> > > +
> > > +               bufp += data_v1->template_data_size;
> > > +       }
> > > +       return ret;
> > > +}
> > > --
> > > 2.7.4
> > >
> > 
> > In overall it is a bit hard to read this function somehow..
> 
> Ok, I'll see if there is any way of simplifying/cleaning up walking the
> measurement list some more.

I moved some code around so that the bufp pointer update is immediately
after the  buffer overflow tests and moved one check outside the while
loop.  I tried defining a buffer overflow macro, but that didn't seem to
help.

The updated patches are available in the next-kexec-restore branch of:
git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git

Mimi

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

end of thread, other threads:[~2016-11-10 13:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-21  2:44 [PATCH v6 00/10] ima: carry the measurement list across kexec Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 01/10] powerpc: ima: Get the kexec buffer passed by the previous kernel Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 02/10] ima: on soft reboot, restore the measurement list Thiago Jung Bauermann
2016-11-08 19:46   ` [Linux-ima-devel] " Dmitry Kasatkin
2016-11-08 20:47     ` Mimi Zohar
2016-11-10 13:12       ` Mimi Zohar
2016-10-21  2:44 ` [PATCH v6 03/10] ima: permit duplicate measurement list entries Thiago Jung Bauermann
2016-11-08 19:47   ` [Linux-ima-devel] " Dmitry Kasatkin
2016-10-21  2:44 ` [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list Thiago Jung Bauermann
2016-11-08 20:05   ` [Linux-ima-devel] " Dmitry Kasatkin
2016-11-08 21:03     ` Mimi Zohar
2016-10-21  2:44 ` [PATCH v6 05/10] powerpc: ima: Send the kexec buffer to the next kernel Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 06/10] ima: on soft reboot, save the measurement list Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list Thiago Jung Bauermann
2016-11-08 23:40   ` [Linux-ima-devel] " Dmitry Kasatkin
2016-10-21  2:44 ` [PATCH v6 08/10] ima: support restoring multiple template formats Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 09/10] ima: define a canonical binary_runtime_measurements list format Thiago Jung Bauermann
2016-10-21  2:44 ` [PATCH v6 10/10] ima: platform-independent hash value Thiago Jung Bauermann

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).