linux-integrity.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
@ 2020-05-04 20:38 Prakhar Srivastava
  2020-05-04 20:38 ` [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-04 20:38 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module
  Cc: catalin.marinas, will, mpe, benh, paulus, robh+dt, frowand.list,
	zohar, dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, mark.rutland,
	masahiroy, james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, prsriva, tusharsu, balajib

IMA during kexec(kexec file load) verifies the kernel signature and measures
the signature of the kernel. The signature in the logs can be used to verfiy the 
authenticity of the kernel. The logs don not get carried over kexec and thus
remote attesation cannot verify the signature of the running kernel.

Introduce an ABI to carry forward the ima logs over kexec.
Memory reserved via device tree reservation can be used to store and read
via the of_* functions.

Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
address, followed by the IMA log contents.

Tested on:
  arm64 with Uboot

Prakhar Srivastava (2):
  Add a layer of abstraction to use the memory reserved by device tree
    for ima buffer pass.
  Add support for ima buffer pass using reserved memory for arm64 kexec.
    Update the arch sepcific code path in kexec file load to store the
    ima buffer in the reserved memory. The same reserved memory is read
    on kexec or cold boot.

 arch/arm64/Kconfig                     |   1 +
 arch/arm64/include/asm/ima.h           |  22 ++++
 arch/arm64/include/asm/kexec.h         |   5 +
 arch/arm64/kernel/Makefile             |   1 +
 arch/arm64/kernel/ima_kexec.c          |  64 ++++++++++
 arch/arm64/kernel/machine_kexec_file.c |   1 +
 arch/powerpc/include/asm/ima.h         |   3 +-
 arch/powerpc/kexec/ima.c               |  14 ++-
 drivers/of/Kconfig                     |   6 +
 drivers/of/Makefile                    |   1 +
 drivers/of/of_ima.c                    | 165 +++++++++++++++++++++++++
 include/linux/of.h                     |  34 +++++
 security/integrity/ima/ima_kexec.c     |  15 ++-
 13 files changed, 325 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c
 create mode 100644 drivers/of/of_ima.c

-- 
2.25.1


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

* [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.
  2020-05-04 20:38 [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Prakhar Srivastava
@ 2020-05-04 20:38 ` Prakhar Srivastava
  2020-05-12 23:09   ` Rob Herring
  2020-05-04 20:38 ` [RFC][PATCH 2/2] Add support for ima buffer pass using reserved memory arm64 Prakhar Srivastava
  2020-05-05  9:59 ` [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Mark Rutland
  2 siblings, 1 reply; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-04 20:38 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module
  Cc: catalin.marinas, will, mpe, benh, paulus, robh+dt, frowand.list,
	zohar, dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, mark.rutland,
	masahiroy, james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, prsriva, tusharsu, balajib

Introduce a device tree layer for to read and store ima buffer
from the reserved memory section of a device tree.

Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
---
 drivers/of/Kconfig  |   6 ++
 drivers/of/Makefile |   1 +
 drivers/of/of_ima.c | 165 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h  |  34 +++++++++
 4 files changed, 206 insertions(+)
 create mode 100644 drivers/of/of_ima.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index d91618641be6..edb3c39740fb 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -107,4 +107,10 @@ config OF_DMA_DEFAULT_COHERENT
 	# arches should select this if DMA is coherent by default for OF devices
 	bool
 
+config OF_IMA
+	def_bool y
+	help
+	  IMA related wrapper functions to add/remove ima measurement logs during
+	  kexec_file_load call.
+
 endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 663a4af0cccd..b4caf083df4e 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
 obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
+obj-$(CONFIG_OF_IMA) += of_ima.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/of_ima.c b/drivers/of/of_ima.c
new file mode 100644
index 000000000000..131f68d81e2e
--- /dev/null
+++ b/drivers/of/of_ima.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Microsoft Corporation.
+ */
+
+#include <linux/slab.h>
+#include <linux/kexec.h>
+#include <linux/of.h>
+#include <linux/memblock.h>
+#include <linux/libfdt.h>
+#include <linux/of_address.h>
+
+static bool dtb_status_enabled;
+static struct resource mem_res;
+static void *vaddr;
+
+
+/**
+ * of_is_ima_memory_reserved - check if memory is reserved via device
+ *							tree.
+ *	Return: zero when memory is not reserved.
+ *			positive number on success.
+ *
+ */
+int of_is_ima_memory_reserved(void)
+{
+	return dtb_status_enabled;
+}
+
+/**
+ * of_ima_write_buffer - Write the ima buffer into the reserved memory.
+ *
+ * ima_buffer - buffer starting address.
+ * ima_buffer_size - size of segment.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int of_ima_write_buffer(void *ima_buffer, size_t ima_buffer_size)
+{
+	void *addr;
+
+	if (!dtb_status_enabled)
+		return -EOPNOTSUPP;
+
+	vaddr = memremap(mem_res.start, resource_size(&mem_res), MEMREMAP_WB);
+	pr_info("Mapped reserved memory, vaddr: 0x%0llX, paddr: 0x%0llX\n , size : %lld",
+	(u64)vaddr, mem_res.start, resource_size(&mem_res));
+
+	if (vaddr) {
+		memcpy(vaddr, &ima_buffer_size, sizeof(size_t));
+		addr =  vaddr + sizeof(size_t);
+		memcpy(addr, ima_buffer, ima_buffer_size);
+		memunmap(vaddr);
+		vaddr = NULL;
+	}
+
+	return 0;
+}
+
+/**
+ * of_remove_ima_buffer - Write 0(Zero length buffer to read)to the
+ *                        size location of the buffer.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int of_remove_ima_buffer(void)
+{
+	size_t empty_buffer_size = 0;
+
+	if (!dtb_status_enabled)
+		return -ENOTSUPP;
+
+	if (vaddr) {
+		memcpy(vaddr, &empty_buffer_size, sizeof(size_t));
+		memunmap(vaddr);
+		vaddr = NULL;
+	}
+
+	return 0;
+}
+
+/**
+ * of_ima_get_size_allocated - Get the usable buffer size thats allocated in
+ *                             the device-tree.
+ *
+ * Return: 0 on unavailable node, size of the memory block - (size_t)
+ */
+size_t of_ima_get_size_allocated(void)
+{
+	size_t size = 0;
+
+	if (!dtb_status_enabled)
+		return size;
+
+	size = resource_size(&mem_res) - sizeof(size_t);
+	return size;
+}
+
+/**
+ * of_get_ima_buffer - Get IMA buffer address.
+ *
+ * @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 of_get_ima_buffer(void **addr, size_t *size)
+{
+	if (!dtb_status_enabled)
+		return -ENOTSUPP;
+
+	vaddr = memremap(mem_res.start, resource_size(&mem_res), MEMREMAP_WB);
+	pr_info("Mapped reserved memory, vaddr: 0x%0llX, paddr: 0x%0llX,\n allocated size : %lld, ima_buffer_size: %ld ",
+	(u64)vaddr, mem_res.start, resource_size(&mem_res), *(size_t *)vaddr);
+
+	*size = *(size_t *)vaddr;
+	*addr = vaddr + sizeof(size_t);
+	return 0;
+}
+
+static const struct of_device_id ima_buffer_pass_ids[] = {
+	{
+		.compatible = "linux,ima_buffer_pass",
+	},
+	{}
+};
+
+static const struct of_device_id ima_buffer_pass_match[] = {
+	{
+		.name = "ima_buffer_pass",
+	},
+};
+MODULE_DEVICE_TABLE(of, ima_buffer_pass_match);
+
+static int __init ima_buffer_pass_init(void)
+{
+	int ret = 0;
+	struct device_node *memnp;
+	struct device_node *ima_buffer_pass_node;
+
+	ima_buffer_pass_node = of_find_matching_node(NULL, ima_buffer_pass_ids);
+	if (!ima_buffer_pass_node)
+		return -ENOENT;
+
+	memnp = of_parse_phandle(ima_buffer_pass_node, "memory-region", 0);
+	if (!memnp)
+		return -ENXIO;
+
+	ret = of_address_to_resource(memnp, 0, &mem_res);
+	if (ret < 0)
+		return -ENOENT;
+
+	of_node_put(memnp);
+	dtb_status_enabled = true;
+
+	return ret;
+}
+
+static void __exit ima_buffer_pass_exit(void)
+{
+	pr_info("trying to exit the ima driver\n");
+}
+
+module_init(ima_buffer_pass_init);
+module_exit(ima_buffer_pass_exit);
diff --git a/include/linux/of.h b/include/linux/of.h
index c669c0a4732f..85ce2f24024f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1485,4 +1485,38 @@ static inline int of_overlay_notifier_unregister(struct notifier_block *nb)
 
 #endif
 
+#ifdef CONFIG_OF_IMA
+int of_is_ima_memory_reserved(void);
+int of_remove_ima_buffer(void);
+int of_get_ima_buffer(void **addr, size_t *size);
+size_t of_ima_get_size_allocated(void);
+int of_ima_write_buffer(void *ima_buffer,
+		size_t ima_buffer_size);
+#else
+static inline int of_is_ima_memory_reserved(void)
+{
+	return -ENOTSUPP;
+};
+static inline int of_remove_ima_buffer(void)
+{
+	return -ENOTSUPP;
+};
+
+static inline int of_get_ima_buffer(void **addr, size_t *size)
+{
+	return -ENOTSUPP;
+};
+
+static inline size_t of_ima_get_size_allocated(void)
+{
+	return 0;
+};
+
+static inline int of_ima_write_buffer(void *ima_buffer,
+				      size_t ima_buffer_size)
+{
+	return -ENOTSUPP;
+};
+#endif
+
 #endif /* _LINUX_OF_H */
-- 
2.25.1


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

* [RFC][PATCH 2/2] Add support for ima buffer pass using reserved memory arm64
  2020-05-04 20:38 [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Prakhar Srivastava
  2020-05-04 20:38 ` [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava
@ 2020-05-04 20:38 ` Prakhar Srivastava
  2020-05-05  9:59 ` [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Mark Rutland
  2 siblings, 0 replies; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-04 20:38 UTC (permalink / raw)
  To: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module
  Cc: catalin.marinas, will, mpe, benh, paulus, robh+dt, frowand.list,
	zohar, dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, mark.rutland,
	masahiroy, james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, prsriva, tusharsu, balajib

 Add support for ima buffer pass using reserved memory for
 arm64 kexec. Update the arch sepcific code path in kexec file load to store
 the ima buffer in the reserved memory. The same reserved memory is read on
 kexec or cold boot.

Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
---
 arch/arm64/Kconfig                     |  1 +
 arch/arm64/include/asm/ima.h           | 22 +++++++++
 arch/arm64/include/asm/kexec.h         |  5 ++
 arch/arm64/kernel/Makefile             |  1 +
 arch/arm64/kernel/ima_kexec.c          | 64 ++++++++++++++++++++++++++
 arch/arm64/kernel/machine_kexec_file.c |  1 +
 arch/powerpc/include/asm/ima.h         |  3 +-
 arch/powerpc/kexec/ima.c               | 14 +++++-
 security/integrity/ima/ima_kexec.c     | 15 ++++--
 9 files changed, 119 insertions(+), 7 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h
 create mode 100644 arch/arm64/kernel/ima_kexec.c

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 40fb05d96c60..bc9e1a91686b 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1069,6 +1069,7 @@ config KEXEC
 config KEXEC_FILE
 	bool "kexec file based system call"
 	select KEXEC_CORE
+	select HAVE_IMA_KEXEC
 	help
 	  This is new version of kexec system call. This system call is
 	  file based and takes file descriptors as system call argument
diff --git a/arch/arm64/include/asm/ima.h b/arch/arm64/include/asm/ima.h
new file mode 100644
index 000000000000..58033b427e59
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,22 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARM64_IMA_H
+#define _ASM_ARM64_IMA_H
+
+struct kimage;
+
+int is_ima_memory_reserved(void);
+int ima_get_kexec_buffer(void **addr, size_t *size);
+int ima_free_kexec_buffer(void);
+
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      void *buffer, size_t size);
+
+#else
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      void *buffer, size_t size)
+{
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARM64_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d24b527e8c00..2bd19ccb6c43 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -100,6 +100,11 @@ struct kimage_arch {
 	void *elf_headers;
 	unsigned long elf_headers_mem;
 	unsigned long elf_headers_sz;
+
+#ifdef CONFIG_IMA_KEXEC
+	phys_addr_t ima_buffer_addr;
+	size_t ima_buffer_size;
+#endif
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 4e5b8ee31442..cd3cb7690d51 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -55,6 +55,7 @@ obj-$(CONFIG_RANDOMIZE_BASE)		+= kaslr.o
 obj-$(CONFIG_HIBERNATION)		+= hibernate.o hibernate-asm.o
 obj-$(CONFIG_KEXEC_CORE)		+= machine_kexec.o relocate_kernel.o	\
 					   cpu-reset.o
+obj-$(CONFIG_HAVE_IMA_KEXEC)		+= ima_kexec.o
 obj-$(CONFIG_KEXEC_FILE)		+= machine_kexec_file.o kexec_image.o
 obj-$(CONFIG_ARM64_RELOC_TEST)		+= arm64-reloc-test.o
 arm64-reloc-test-y := reloc_test_core.o reloc_test_syms.o
diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c
new file mode 100644
index 000000000000..ff5649333c7c
--- /dev/null
+++ b/arch/arm64/kernel/ima_kexec.c
@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Microsoft Corporation.
+ *
+ * Authors:
+ * Prakhar Srivastava <prsriva@linux.microsoft.com>
+ */
+
+#include <linux/kexec.h>
+#include <linux/of.h>
+
+
+/**
+ * is_ima_memory_reserved - check if memory is reserved via device
+ *			    tree.
+ *	Return: negative or zero when memory is not reserved.
+ *	positive number on success.
+ *
+ */
+int is_ima_memory_reserved(void)
+{
+	return of_is_ima_memory_reserved();
+}
+
+/**
+ * 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)
+{
+	return of_get_ima_buffer(addr, size);
+}
+
+/**
+ * ima_free_kexec_buffer - free memory used by the IMA buffer
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int ima_free_kexec_buffer(void)
+{
+	return of_remove_ima_buffer();
+}
+
+#ifdef CONFIG_IMA_KEXEC
+/**
+ * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA
+ *	measurement log.
+ * @image: - pointer to the kimage, to store the address and size of the
+ *	IMA measurement log.
+ * @load_addr: - the address where the IMA measurement log is stored.
+ * @size - size of the IMA measurement log.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+			      void *buffer, size_t size)
+{
+	of_ima_write_buffer(buffer, size);
+	return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
index b40c3b0def92..8dc25511142d 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -22,6 +22,7 @@
 #include <linux/types.h>
 #include <linux/vmalloc.h>
 #include <asm/byteorder.h>
+#include <asm/ima.h>
 
 /* relevant device tree properties */
 #define FDT_PROP_KEXEC_ELFHDR	"linux,elfcorehdr"
diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
index ead488cf3981..a8febc620b42 100644
--- a/arch/powerpc/include/asm/ima.h
+++ b/arch/powerpc/include/asm/ima.h
@@ -4,6 +4,7 @@
 
 struct kimage;
 
+int is_ima_memory_reserved(void);
 int ima_get_kexec_buffer(void **addr, size_t *size);
 int ima_free_kexec_buffer(void);
 
@@ -15,7 +16,7 @@ static inline void remove_ima_buffer(void *fdt, int chosen_node) {}
 
 #ifdef CONFIG_IMA_KEXEC
 int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
-			      size_t size);
+			      void *buffer, size_t size);
 
 int setup_ima_buffer(const struct kimage *image, void *fdt, int chosen_node);
 #else
diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
index 720e50e490b6..3823539d4e07 100644
--- a/arch/powerpc/kexec/ima.c
+++ b/arch/powerpc/kexec/ima.c
@@ -46,6 +46,18 @@ static int do_get_kexec_buffer(const void *prop, int len, unsigned long *addr,
 	return 0;
 }
 
+/**
+ * is_ima_memory_reserved - check if memory is reserved via device
+ *			    tree.
+ *	Return: negative or zero when memory is not reserved.
+ *	positive number on success.
+ *
+ */
+int is_ima_memory_reserved(void)
+{
+	return -EOPNOTSUPP;
+}
+
 /**
  * ima_get_kexec_buffer - get IMA buffer from the previous kernel
  * @addr:	On successful return, set to point to the buffer contents.
@@ -137,7 +149,7 @@ void remove_ima_buffer(void *fdt, int chosen_node)
  * Return: 0 on success, negative errno on error.
  */
 int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
-			      size_t size)
+			      void *buffer, size_t size)
 {
 	image->arch.ima_buffer_addr = load_addr;
 	image->arch.ima_buffer_size = size;
diff --git a/security/integrity/ima/ima_kexec.c b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..3749472c7e18 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -116,13 +116,18 @@ void ima_add_kexec_buffer(struct kimage *image)
 	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;
+
+	if (!is_ima_memory_reserved()) {
+
+		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);
+	ret = arch_ima_add_kexec_buffer(image, kbuf.mem, kexec_buffer,
+					kexec_segment_size);
 	if (ret) {
 		pr_err("Error passing over kexec measurement buffer.\n");
 		return;
-- 
2.25.1


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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-04 20:38 [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Prakhar Srivastava
  2020-05-04 20:38 ` [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava
  2020-05-04 20:38 ` [RFC][PATCH 2/2] Add support for ima buffer pass using reserved memory arm64 Prakhar Srivastava
@ 2020-05-05  9:59 ` Mark Rutland
  2020-05-07  5:50   ` Prakhar Srivastava
  2 siblings, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2020-05-05  9:59 UTC (permalink / raw)
  To: Prakhar Srivastava
  Cc: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module, catalin.marinas, will,
	mpe, benh, paulus, robh+dt, frowand.list, zohar, dmitry.kasatkin,
	jmorris, serge, pasha.tatashin, allison, kstewart,
	takahiro.akashi, tglx, vincenzo.frascino, masahiroy, james.morse,
	bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy, gregkh,
	nramas, tusharsu, balajib

Hi Prakhar,

On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
> IMA during kexec(kexec file load) verifies the kernel signature and measures
> the signature of the kernel. The signature in the logs can be used to verfiy the 
> authenticity of the kernel. The logs don not get carried over kexec and thus
> remote attesation cannot verify the signature of the running kernel.
> 
> Introduce an ABI to carry forward the ima logs over kexec.
> Memory reserved via device tree reservation can be used to store and read
> via the of_* functions.

This flow needs to work for:

1) Pure DT
2) DT + EFI memory map
3) ACPI + EFI memory map

... and if this is just for transiently passing the log, I don't think
that a reserved memory region is the right thing to use, since they're
supposed to be more permanent.

This sounds analogous to passing the initrd, and should probably use
properties under the chosen node (which can be used for all three boot
flows above).

For reference, how big is the IMA log likely to be? Does it need
physically contiguous space?

Thanks,
Mark.

> 
> Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
> address, followed by the IMA log contents.
> 
> Tested on:
>   arm64 with Uboot
> 
> Prakhar Srivastava (2):
>   Add a layer of abstraction to use the memory reserved by device tree
>     for ima buffer pass.
>   Add support for ima buffer pass using reserved memory for arm64 kexec.
>     Update the arch sepcific code path in kexec file load to store the
>     ima buffer in the reserved memory. The same reserved memory is read
>     on kexec or cold boot.
> 
>  arch/arm64/Kconfig                     |   1 +
>  arch/arm64/include/asm/ima.h           |  22 ++++
>  arch/arm64/include/asm/kexec.h         |   5 +
>  arch/arm64/kernel/Makefile             |   1 +
>  arch/arm64/kernel/ima_kexec.c          |  64 ++++++++++
>  arch/arm64/kernel/machine_kexec_file.c |   1 +
>  arch/powerpc/include/asm/ima.h         |   3 +-
>  arch/powerpc/kexec/ima.c               |  14 ++-
>  drivers/of/Kconfig                     |   6 +
>  drivers/of/Makefile                    |   1 +
>  drivers/of/of_ima.c                    | 165 +++++++++++++++++++++++++
>  include/linux/of.h                     |  34 +++++
>  security/integrity/ima/ima_kexec.c     |  15 ++-
>  13 files changed, 325 insertions(+), 7 deletions(-)
>  create mode 100644 arch/arm64/include/asm/ima.h
>  create mode 100644 arch/arm64/kernel/ima_kexec.c
>  create mode 100644 drivers/of/of_ima.c
> 
> -- 
> 2.25.1
> 

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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-05  9:59 ` [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Mark Rutland
@ 2020-05-07  5:50   ` Prakhar Srivastava
  2020-05-12 23:05     ` Rob Herring
  0 siblings, 1 reply; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-07  5:50 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module, catalin.marinas, will,
	mpe, benh, paulus, robh+dt, frowand.list, zohar, dmitry.kasatkin,
	jmorris, serge, pasha.tatashin, allison, kstewart,
	takahiro.akashi, tglx, vincenzo.frascino, masahiroy, james.morse,
	bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy, gregkh,
	nramas, tusharsu, balajib

Hi Mark,

This patch set currently only address the Pure DT implementation.
EFI and ACPI implementations will be posted in subsequent patchsets.

The logs are intended to be carried over the kexec and once read the
logs are no longer needed and in prior conversation with James(
https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/) 
the apporach of using a chosen node doesn't
support the case.

The DT entries make the reservation permanent and thus doesnt need 
kernel segments to be used for this, however using a chosen-node with
reserved memory only changes the node information but memory still is
reserved via reserved-memory section.

On 5/5/20 2:59 AM, Mark Rutland wrote:
> Hi Prakhar,
> 
> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>> the signature of the kernel. The signature in the logs can be used to verfiy the
>> authenticity of the kernel. The logs don not get carried over kexec and thus
>> remote attesation cannot verify the signature of the running kernel.
>>
>> Introduce an ABI to carry forward the ima logs over kexec.
>> Memory reserved via device tree reservation can be used to store and read
>> via the of_* functions.
> 
> This flow needs to work for:
> 
> 1) Pure DT
> 2) DT + EFI memory map
> 3) ACPI + EFI memory map
> 
> ... and if this is just for transiently passing the log, I don't think
> that a reserved memory region is the right thing to use, since they're
> supposed to be more permanent.
> 
> This sounds analogous to passing the initrd, and should probably use
> properties under the chosen node (which can be used for all three boot
> flows above).
> 
> For reference, how big is the IMA log likely to be? Does it need
> physically contiguous space?

It purely depends on the policy used and the modules/files that are 
accessed for my local testing over a kexec session the log in
about 30KB.

Current implementation expects enough contiguous memory to allocated to 
carry forward the logs. If the log size exceeds the reserved memory the
call will fail.

Thanks,
Prakhar Srivastava
> 
> Thanks,
> Mark.
> 
>>
>> Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
>> address, followed by the IMA log contents.
>>
>> Tested on:
>>    arm64 with Uboot
>>
>> Prakhar Srivastava (2):
>>    Add a layer of abstraction to use the memory reserved by device tree
>>      for ima buffer pass.
>>    Add support for ima buffer pass using reserved memory for arm64 kexec.
>>      Update the arch sepcific code path in kexec file load to store the
>>      ima buffer in the reserved memory. The same reserved memory is read
>>      on kexec or cold boot.
>>
>>   arch/arm64/Kconfig                     |   1 +
>>   arch/arm64/include/asm/ima.h           |  22 ++++
>>   arch/arm64/include/asm/kexec.h         |   5 +
>>   arch/arm64/kernel/Makefile             |   1 +
>>   arch/arm64/kernel/ima_kexec.c          |  64 ++++++++++
>>   arch/arm64/kernel/machine_kexec_file.c |   1 +
>>   arch/powerpc/include/asm/ima.h         |   3 +-
>>   arch/powerpc/kexec/ima.c               |  14 ++-
>>   drivers/of/Kconfig                     |   6 +
>>   drivers/of/Makefile                    |   1 +
>>   drivers/of/of_ima.c                    | 165 +++++++++++++++++++++++++
>>   include/linux/of.h                     |  34 +++++
>>   security/integrity/ima/ima_kexec.c     |  15 ++-
>>   13 files changed, 325 insertions(+), 7 deletions(-)
>>   create mode 100644 arch/arm64/include/asm/ima.h
>>   create mode 100644 arch/arm64/kernel/ima_kexec.c
>>   create mode 100644 drivers/of/of_ima.c
>>
>> -- 
>> 2.25.1
>>

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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-07  5:50   ` Prakhar Srivastava
@ 2020-05-12 23:05     ` Rob Herring
  2020-05-18 20:16       ` Prakhar Srivastava
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-05-12 23:05 UTC (permalink / raw)
  To: Prakhar Srivastava
  Cc: Mark Rutland, linux-arm-kernel, linux-kernel, linuxppc-dev,
	devicetree, linux-integrity, linux-security-module,
	catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
	dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
	james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, tusharsu, balajib

On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
> Hi Mark,

Please don't top post.

> This patch set currently only address the Pure DT implementation.
> EFI and ACPI implementations will be posted in subsequent patchsets.
> 
> The logs are intended to be carried over the kexec and once read the
> logs are no longer needed and in prior conversation with James(
> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
> the apporach of using a chosen node doesn't
> support the case.
> 
> The DT entries make the reservation permanent and thus doesnt need kernel
> segments to be used for this, however using a chosen-node with
> reserved memory only changes the node information but memory still is
> reserved via reserved-memory section.

I think Mark's point was whether it needs to be permanent. We don't 
hardcode the initrd address for example.

> On 5/5/20 2:59 AM, Mark Rutland wrote:
> > Hi Prakhar,
> > 
> > On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
> > > IMA during kexec(kexec file load) verifies the kernel signature and measures

What's IMA?

> > > the signature of the kernel. The signature in the logs can be used to verfiy the
> > > authenticity of the kernel. The logs don not get carried over kexec and thus
> > > remote attesation cannot verify the signature of the running kernel.
> > > 
> > > Introduce an ABI to carry forward the ima logs over kexec.
> > > Memory reserved via device tree reservation can be used to store and read
> > > via the of_* functions.
> > 
> > This flow needs to work for:
> > 
> > 1) Pure DT
> > 2) DT + EFI memory map
> > 3) ACPI + EFI memory map
> > 
> > ... and if this is just for transiently passing the log, I don't think
> > that a reserved memory region is the right thing to use, since they're
> > supposed to be more permanent.
> > 
> > This sounds analogous to passing the initrd, and should probably use
> > properties under the chosen node (which can be used for all three boot
> > flows above).
> > 
> > For reference, how big is the IMA log likely to be? Does it need
> > physically contiguous space?
> 
> It purely depends on the policy used and the modules/files that are accessed
> for my local testing over a kexec session the log in
> about 30KB.
> 
> Current implementation expects enough contiguous memory to allocated to
> carry forward the logs. If the log size exceeds the reserved memory the
> call will fail.
> 
> Thanks,
> Prakhar Srivastava
> > 
> > Thanks,
> > Mark.
> > 
> > > 
> > > Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
> > > address, followed by the IMA log contents.
> > > 
> > > Tested on:
> > >    arm64 with Uboot
> > > 
> > > Prakhar Srivastava (2):
> > >    Add a layer of abstraction to use the memory reserved by device tree
> > >      for ima buffer pass.
> > >    Add support for ima buffer pass using reserved memory for arm64 kexec.
> > >      Update the arch sepcific code path in kexec file load to store the
> > >      ima buffer in the reserved memory. The same reserved memory is read
> > >      on kexec or cold boot.
> > > 
> > >   arch/arm64/Kconfig                     |   1 +
> > >   arch/arm64/include/asm/ima.h           |  22 ++++
> > >   arch/arm64/include/asm/kexec.h         |   5 +
> > >   arch/arm64/kernel/Makefile             |   1 +
> > >   arch/arm64/kernel/ima_kexec.c          |  64 ++++++++++
> > >   arch/arm64/kernel/machine_kexec_file.c |   1 +
> > >   arch/powerpc/include/asm/ima.h         |   3 +-
> > >   arch/powerpc/kexec/ima.c               |  14 ++-
> > >   drivers/of/Kconfig                     |   6 +
> > >   drivers/of/Makefile                    |   1 +
> > >   drivers/of/of_ima.c                    | 165 +++++++++++++++++++++++++
> > >   include/linux/of.h                     |  34 +++++
> > >   security/integrity/ima/ima_kexec.c     |  15 ++-
> > >   13 files changed, 325 insertions(+), 7 deletions(-)
> > >   create mode 100644 arch/arm64/include/asm/ima.h
> > >   create mode 100644 arch/arm64/kernel/ima_kexec.c
> > >   create mode 100644 drivers/of/of_ima.c
> > > 
> > > -- 
> > > 2.25.1
> > > 

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

* Re: [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.
  2020-05-04 20:38 ` [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava
@ 2020-05-12 23:09   ` Rob Herring
  2020-05-18 20:34     ` Prakhar Srivastava
  0 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2020-05-12 23:09 UTC (permalink / raw)
  To: Prakhar Srivastava
  Cc: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module, catalin.marinas, will,
	mpe, benh, paulus, frowand.list, zohar, dmitry.kasatkin, jmorris,
	serge, pasha.tatashin, allison, kstewart, takahiro.akashi, tglx,
	vincenzo.frascino, mark.rutland, masahiroy, james.morse,
	bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy, gregkh,
	nramas, tusharsu, balajib

On Mon, May 04, 2020 at 01:38:28PM -0700, Prakhar Srivastava wrote:
> Introduce a device tree layer for to read and store ima buffer
> from the reserved memory section of a device tree.

But why do I need 'a layer of abstraction'? I don't like them.

> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
> ---
>  drivers/of/Kconfig  |   6 ++
>  drivers/of/Makefile |   1 +
>  drivers/of/of_ima.c | 165 ++++++++++++++++++++++++++++++++++++++++++++

Who are the users of this code and why does it need to be here? Most 
code for specific bindings are not in drivers/of/ but with the user. It 
doesn't sound like there's more than 1 user.

>  include/linux/of.h  |  34 +++++++++
>  4 files changed, 206 insertions(+)
>  create mode 100644 drivers/of/of_ima.c

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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-12 23:05     ` Rob Herring
@ 2020-05-18 20:16       ` Prakhar Srivastava
  2020-05-23  4:08         ` Thiago Jung Bauermann
  0 siblings, 1 reply; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-18 20:16 UTC (permalink / raw)
  To: Rob Herring
  Cc: Mark Rutland, linux-arm-kernel, linux-kernel, linuxppc-dev,
	devicetree, linux-integrity, linux-security-module,
	catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
	dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
	james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, tusharsu, balajib



On 5/12/20 4:05 PM, Rob Herring wrote:
> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>> Hi Mark,
> 
> Please don't top post.
> 
>> This patch set currently only address the Pure DT implementation.
>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>
>> The logs are intended to be carried over the kexec and once read the
>> logs are no longer needed and in prior conversation with James(
>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>> the apporach of using a chosen node doesn't
>> support the case.
>>
>> The DT entries make the reservation permanent and thus doesnt need kernel
>> segments to be used for this, however using a chosen-node with
>> reserved memory only changes the node information but memory still is
>> reserved via reserved-memory section.
> 
> I think Mark's point was whether it needs to be permanent. We don't
> hardcode the initrd address for example.
> 
Thankyou for clarifying my misunderstanding, i am modelling this keeping 
to the TPM log implementation that uses a reserved memory. I will rev up 
the version with chosen-node support.
That will make the memory reservation free after use.


>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>> Hi Prakhar,
>>>
>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
> 
> What's IMAIMA is a LSM attempting to detect if files have been accidentally or 
maliciously altered, both remotely and locally, it can also be used
to appraise a file's measurement against a "good" value stored as an 
extended attribute, and enforce local file integrity.

IMA also validates and measures the signers of the kernel and initrd
during kexec. The measurements are extended to PCR 10(configurable) and 
the logs stored in memory, however once kexec'd the logs are lost. Kexec 
is used as secondary boot loader in may use cases and loosing the signer
creates a security hole.

This patch is an implementation to carry over the logs and making it
possible to remotely validate the signers of the kernel and initrd. Such 
a support exits only in powerpc.
This patch makes the carry over of logs architecture independent and 
puts the complexity in a driver.

Thanks,
Prakhar
> 
>>>> the signature of the kernel. The signature in the logs can be used to verfiy the
>>>> authenticity of the kernel. The logs don not get carried over kexec and thus
>>>> remote attesation cannot verify the signature of the running kernel.
>>>>
>>>> Introduce an ABI to carry forward the ima logs over kexec.
>>>> Memory reserved via device tree reservation can be used to store and read
>>>> via the of_* functions.
>>>
>>> This flow needs to work for:
>>>
>>> 1) Pure DT
>>> 2) DT + EFI memory map
>>> 3) ACPI + EFI memory map
>>>
>>> ... and if this is just for transiently passing the log, I don't think
>>> that a reserved memory region is the right thing to use, since they're
>>> supposed to be more permanent.
>>>
>>> This sounds analogous to passing the initrd, and should probably use
>>> properties under the chosen node (which can be used for all three boot
>>> flows above).
>>>
>>> For reference, how big is the IMA log likely to be? Does it need
>>> physically contiguous space?
>>
>> It purely depends on the policy used and the modules/files that are accessed
>> for my local testing over a kexec session the log in
>> about 30KB.
>>
>> Current implementation expects enough contiguous memory to allocated to
>> carry forward the logs. If the log size exceeds the reserved memory the
>> call will fail.
>>
>> Thanks,
>> Prakhar Srivastava
>>>
>>> Thanks,
>>> Mark.
>>>
>>>>
>>>> Reserved memory stores the size(sizeof(size_t)) of the buffer in the starting
>>>> address, followed by the IMA log contents.
>>>>
>>>> Tested on:
>>>>     arm64 with Uboot
>>>>
>>>> Prakhar Srivastava (2):
>>>>     Add a layer of abstraction to use the memory reserved by device tree
>>>>       for ima buffer pass.
>>>>     Add support for ima buffer pass using reserved memory for arm64 kexec.
>>>>       Update the arch sepcific code path in kexec file load to store the
>>>>       ima buffer in the reserved memory. The same reserved memory is read
>>>>       on kexec or cold boot.
>>>>
>>>>    arch/arm64/Kconfig                     |   1 +
>>>>    arch/arm64/include/asm/ima.h           |  22 ++++
>>>>    arch/arm64/include/asm/kexec.h         |   5 +
>>>>    arch/arm64/kernel/Makefile             |   1 +
>>>>    arch/arm64/kernel/ima_kexec.c          |  64 ++++++++++
>>>>    arch/arm64/kernel/machine_kexec_file.c |   1 +
>>>>    arch/powerpc/include/asm/ima.h         |   3 +-
>>>>    arch/powerpc/kexec/ima.c               |  14 ++-
>>>>    drivers/of/Kconfig                     |   6 +
>>>>    drivers/of/Makefile                    |   1 +
>>>>    drivers/of/of_ima.c                    | 165 +++++++++++++++++++++++++
>>>>    include/linux/of.h                     |  34 +++++
>>>>    security/integrity/ima/ima_kexec.c     |  15 ++-
>>>>    13 files changed, 325 insertions(+), 7 deletions(-)
>>>>    create mode 100644 arch/arm64/include/asm/ima.h
>>>>    create mode 100644 arch/arm64/kernel/ima_kexec.c
>>>>    create mode 100644 drivers/of/of_ima.c
>>>>
>>>> -- 
>>>> 2.25.1
>>>>

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

* Re: [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.
  2020-05-12 23:09   ` Rob Herring
@ 2020-05-18 20:34     ` Prakhar Srivastava
  0 siblings, 0 replies; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-18 20:34 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-arm-kernel, linux-kernel, linuxppc-dev, devicetree,
	linux-integrity, linux-security-module, catalin.marinas, will,
	mpe, benh, paulus, frowand.list, zohar, dmitry.kasatkin, jmorris,
	serge, pasha.tatashin, allison, kstewart, takahiro.akashi, tglx,
	vincenzo.frascino, mark.rutland, masahiroy, james.morse,
	bhsharma, mbrugger, hsinyi, tao.li, christophe.leroy, gregkh,
	nramas, tusharsu, balajib



On 5/12/20 4:09 PM, Rob Herring wrote:
> On Mon, May 04, 2020 at 01:38:28PM -0700, Prakhar Srivastava wrote:
>> Introduce a device tree layer for to read and store ima buffer
>> from the reserved memory section of a device tree.
> 
> But why do I need 'a layer of abstraction'? I don't like them.
> 
This is a common path for the all architectures to carry over the
IMA measurement logs. A single layer will avoid any code duplication.

>> Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
>> ---
>>   drivers/of/Kconfig  |   6 ++
>>   drivers/of/Makefile |   1 +
>>   drivers/of/of_ima.c | 165 ++++++++++++++++++++++++++++++++++++++++++++
> 
> Who are the users of this code and why does it need to be here? Most
> code for specific bindings are not in drivers/of/ but with the user. It
> doesn't sound like there's more than 1 user.
> 
Currently the path is exercised by arm64 kexec_file_load path. A slight
restructuring is needed on the powerpc side to use the same code path 
and other architectures can follow to add carrying over IMA logs over
kexec with just a few function calls.

I have attempted to bring the code path down to the highest common 
layer, however please do suggest if i can move this some where else.

Thanks,
Prakhar

>>   include/linux/of.h  |  34 +++++++++
>>   4 files changed, 206 insertions(+)
>>   create mode 100644 drivers/of/of_ima.c

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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-18 20:16       ` Prakhar Srivastava
@ 2020-05-23  4:08         ` Thiago Jung Bauermann
  2020-06-01  4:05           ` Prakhar Srivastava
  0 siblings, 1 reply; 12+ messages in thread
From: Thiago Jung Bauermann @ 2020-05-23  4:08 UTC (permalink / raw)
  To: Prakhar Srivastava
  Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
	linuxppc-dev, devicetree, linux-integrity, linux-security-module,
	catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
	dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
	james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, tusharsu, balajib


Hello Prakhar,

Prakhar Srivastava <prsriva@linux.microsoft.com> writes:

> On 5/12/20 4:05 PM, Rob Herring wrote:
>> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>>> Hi Mark,
>>
>> Please don't top post.
>>
>>> This patch set currently only address the Pure DT implementation.
>>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>>
>>> The logs are intended to be carried over the kexec and once read the
>>> logs are no longer needed and in prior conversation with James(
>>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>>> the apporach of using a chosen node doesn't
>>> support the case.
>>>
>>> The DT entries make the reservation permanent and thus doesnt need kernel
>>> segments to be used for this, however using a chosen-node with
>>> reserved memory only changes the node information but memory still is
>>> reserved via reserved-memory section.
>>
>> I think Mark's point was whether it needs to be permanent. We don't
>> hardcode the initrd address for example.
>>
> Thankyou for clarifying my misunderstanding, i am modelling this keeping to the
> TPM log implementation that uses a reserved memory. I will rev up the version
> with chosen-node support.
> That will make the memory reservation free after use.

Nice. Do you intend to use the same property that powerpc uses
(linux,ima-kexec-buffer)?

>>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>>> Hi Prakhar,
>>>>
>>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>>
>> What's IMAIMA is a LSM attempting to detect if files have been accidentally or
> maliciously altered, both remotely and locally, it can also be used
> to appraise a file's measurement against a "good" value stored as an extended
> attribute, and enforce local file integrity.
>
> IMA also validates and measures the signers of the kernel and initrd
> during kexec. The measurements are extended to PCR 10(configurable) and the logs
> stored in memory, however once kexec'd the logs are lost. Kexec is used as
> secondary boot loader in may use cases and loosing the signer
> creates a security hole.
>
> This patch is an implementation to carry over the logs and making it
> possible to remotely validate the signers of the kernel and initrd. Such a
> support exits only in powerpc.
> This patch makes the carry over of logs architecture independent and puts the
> complexity in a driver.

If I'm not mistaken, the code at arch/powerpc/kexec/ima.c isn't actually
powerpc-specific. It could be moved to an arch-independent directory and
used by any other architecture which supports device trees.

I think that's the simplest way forward. And to be honest I'm still
trying to understand why you didn't take that approach. Did you try it
and hit some obstacle or noticed a disadvantage for your use case?

--
Thiago Jung Bauermann
IBM Linux Technology Center

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

* Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass
  2020-05-23  4:08         ` Thiago Jung Bauermann
@ 2020-06-01  4:05           ` Prakhar Srivastava
  0 siblings, 0 replies; 12+ messages in thread
From: Prakhar Srivastava @ 2020-06-01  4:05 UTC (permalink / raw)
  To: Thiago Jung Bauermann
  Cc: Rob Herring, Mark Rutland, linux-arm-kernel, linux-kernel,
	linuxppc-dev, devicetree, linux-integrity, linux-security-module,
	catalin.marinas, will, mpe, benh, paulus, frowand.list, zohar,
	dmitry.kasatkin, jmorris, serge, pasha.tatashin, allison,
	kstewart, takahiro.akashi, tglx, vincenzo.frascino, masahiroy,
	james.morse, bhsharma, mbrugger, hsinyi, tao.li,
	christophe.leroy, gregkh, nramas, tusharsu, balajib


On 5/22/20 9:08 PM, Thiago Jung Bauermann wrote:
> 
> Hello Prakhar,
> 
> Prakhar Srivastava <prsriva@linux.microsoft.com> writes:
> 
>> On 5/12/20 4:05 PM, Rob Herring wrote:
>>> On Wed, May 06, 2020 at 10:50:04PM -0700, Prakhar Srivastava wrote:
>>>> Hi Mark,
>>>
>>> Please don't top post.
>>>
>>>> This patch set currently only address the Pure DT implementation.
>>>> EFI and ACPI implementations will be posted in subsequent patchsets.
>>>>
>>>> The logs are intended to be carried over the kexec and once read the
>>>> logs are no longer needed and in prior conversation with James(
>>>> https://lore.kernel.org/linux-arm-kernel/0053eb68-0905-4679-c97a-00c5cb6f1abb@arm.com/)
>>>> the apporach of using a chosen node doesn't
>>>> support the case.
>>>>
>>>> The DT entries make the reservation permanent and thus doesnt need kernel
>>>> segments to be used for this, however using a chosen-node with
>>>> reserved memory only changes the node information but memory still is
>>>> reserved via reserved-memory section.
>>>
>>> I think Mark's point was whether it needs to be permanent. We don't
>>> hardcode the initrd address for example.
>>>
>> Thankyou for clarifying my misunderstanding, i am modelling this keeping to the
>> TPM log implementation that uses a reserved memory. I will rev up the version
>> with chosen-node support.
>> That will make the memory reservation free after use.
> 
> Nice. Do you intend to use the same property that powerpc uses
> (linux,ima-kexec-buffer)?
> 
I was naming it ima-buffer, but naming is not a huge concern.
I will use linux,ima-kexec-buffer.
>>>> On 5/5/20 2:59 AM, Mark Rutland wrote:
>>>>> Hi Prakhar,
>>>>>
>>>>> On Mon, May 04, 2020 at 01:38:27PM -0700, Prakhar Srivastava wrote:
>>>>>> IMA during kexec(kexec file load) verifies the kernel signature and measures
>>>
>>> What's IMAIMA is a LSM attempting to detect if files have been accidentally or
>> maliciously altered, both remotely and locally, it can also be used
>> to appraise a file's measurement against a "good" value stored as an extended
>> attribute, and enforce local file integrity.
>>
>> IMA also validates and measures the signers of the kernel and initrd
>> during kexec. The measurements are extended to PCR 10(configurable) and the logs
>> stored in memory, however once kexec'd the logs are lost. Kexec is used as
>> secondary boot loader in may use cases and loosing the signer
>> creates a security hole.
>>
>> This patch is an implementation to carry over the logs and making it
>> possible to remotely validate the signers of the kernel and initrd. Such a
>> support exits only in powerpc.
>> This patch makes the carry over of logs architecture independent and puts the
>> complexity in a driver.
> 
> If I'm not mistaken, the code at arch/powerpc/kexec/ima.c isn't actually
> powerpc-specific. It could be moved to an arch-independent directory and
> used by any other architecture which supports device trees.
> 
> I think that's the simplest way forward. And to be honest I'm still
> trying to understand why you didn't take that approach. Did you try it
> and hit some obstacle or noticed a disadvantage for your use case?
> 
The approach i have in this patch set is to provide an abstraction layer 
that can be called from any architecture.
However taking a deeper look only the setup dtb is probably architecture
specific, only because the architecture specific kexec sets up the 
device tree. I can also move the code up in security/ima. However i do
have some concerns with layering. I am hoping you can provide me with 
some guidance in this aspect, i will send you the patch i am working on
to get some early feedback.

Thanks,
Prakhar Srivastava


> --
> Thiago Jung Bauermann
> IBM Linux Technology Center
> 

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

* [RFC]{PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.
  2020-05-01 18:15 Prakhar Srivastava
@ 2020-05-01 18:15 ` Prakhar Srivastava
  0 siblings, 0 replies; 12+ messages in thread
From: Prakhar Srivastava @ 2020-05-01 18:15 UTC (permalink / raw)
  To: linux-integrity, linux-kernel, devicetree
  Cc: nramas, prsriva, tusharsu, balajib

Introduce a device tree layer for to read and store ima buffer
from the reserved memory section of a device tree.

Signed-off-by: Prakhar Srivastava <prsriva@linux.microsoft.com>
---
 drivers/of/Kconfig  |   6 ++
 drivers/of/Makefile |   1 +
 drivers/of/of_ima.c | 165 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/of.h  |  34 +++++++++
 4 files changed, 206 insertions(+)
 create mode 100644 drivers/of/of_ima.c

diff --git a/drivers/of/Kconfig b/drivers/of/Kconfig
index d91618641be6..edb3c39740fb 100644
--- a/drivers/of/Kconfig
+++ b/drivers/of/Kconfig
@@ -107,4 +107,10 @@ config OF_DMA_DEFAULT_COHERENT
 	# arches should select this if DMA is coherent by default for OF devices
 	bool
 
+config OF_IMA
+	def_bool y
+	help
+	  IMA related wrapper functions to add/remove ima measurement logs during
+	  kexec_file_load call.
+
 endif # OF
diff --git a/drivers/of/Makefile b/drivers/of/Makefile
index 663a4af0cccd..b4caf083df4e 100644
--- a/drivers/of/Makefile
+++ b/drivers/of/Makefile
@@ -14,5 +14,6 @@ obj-$(CONFIG_OF_RESERVED_MEM) += of_reserved_mem.o
 obj-$(CONFIG_OF_RESOLVE)  += resolver.o
 obj-$(CONFIG_OF_OVERLAY) += overlay.o
 obj-$(CONFIG_OF_NUMA) += of_numa.o
+obj-$(CONFIG_OF_IMA) += of_ima.o
 
 obj-$(CONFIG_OF_UNITTEST) += unittest-data/
diff --git a/drivers/of/of_ima.c b/drivers/of/of_ima.c
new file mode 100644
index 000000000000..131f68d81e2e
--- /dev/null
+++ b/drivers/of/of_ima.c
@@ -0,0 +1,165 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2020 Microsoft Corporation.
+ */
+
+#include <linux/slab.h>
+#include <linux/kexec.h>
+#include <linux/of.h>
+#include <linux/memblock.h>
+#include <linux/libfdt.h>
+#include <linux/of_address.h>
+
+static bool dtb_status_enabled;
+static struct resource mem_res;
+static void *vaddr;
+
+
+/**
+ * of_is_ima_memory_reserved - check if memory is reserved via device
+ *							tree.
+ *	Return: zero when memory is not reserved.
+ *			positive number on success.
+ *
+ */
+int of_is_ima_memory_reserved(void)
+{
+	return dtb_status_enabled;
+}
+
+/**
+ * of_ima_write_buffer - Write the ima buffer into the reserved memory.
+ *
+ * ima_buffer - buffer starting address.
+ * ima_buffer_size - size of segment.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int of_ima_write_buffer(void *ima_buffer, size_t ima_buffer_size)
+{
+	void *addr;
+
+	if (!dtb_status_enabled)
+		return -EOPNOTSUPP;
+
+	vaddr = memremap(mem_res.start, resource_size(&mem_res), MEMREMAP_WB);
+	pr_info("Mapped reserved memory, vaddr: 0x%0llX, paddr: 0x%0llX\n , size : %lld",
+	(u64)vaddr, mem_res.start, resource_size(&mem_res));
+
+	if (vaddr) {
+		memcpy(vaddr, &ima_buffer_size, sizeof(size_t));
+		addr =  vaddr + sizeof(size_t);
+		memcpy(addr, ima_buffer, ima_buffer_size);
+		memunmap(vaddr);
+		vaddr = NULL;
+	}
+
+	return 0;
+}
+
+/**
+ * of_remove_ima_buffer - Write 0(Zero length buffer to read)to the
+ *                        size location of the buffer.
+ *
+ * Return: 0 on success, negative errno on error.
+ */
+int of_remove_ima_buffer(void)
+{
+	size_t empty_buffer_size = 0;
+
+	if (!dtb_status_enabled)
+		return -ENOTSUPP;
+
+	if (vaddr) {
+		memcpy(vaddr, &empty_buffer_size, sizeof(size_t));
+		memunmap(vaddr);
+		vaddr = NULL;
+	}
+
+	return 0;
+}
+
+/**
+ * of_ima_get_size_allocated - Get the usable buffer size thats allocated in
+ *                             the device-tree.
+ *
+ * Return: 0 on unavailable node, size of the memory block - (size_t)
+ */
+size_t of_ima_get_size_allocated(void)
+{
+	size_t size = 0;
+
+	if (!dtb_status_enabled)
+		return size;
+
+	size = resource_size(&mem_res) - sizeof(size_t);
+	return size;
+}
+
+/**
+ * of_get_ima_buffer - Get IMA buffer address.
+ *
+ * @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 of_get_ima_buffer(void **addr, size_t *size)
+{
+	if (!dtb_status_enabled)
+		return -ENOTSUPP;
+
+	vaddr = memremap(mem_res.start, resource_size(&mem_res), MEMREMAP_WB);
+	pr_info("Mapped reserved memory, vaddr: 0x%0llX, paddr: 0x%0llX,\n allocated size : %lld, ima_buffer_size: %ld ",
+	(u64)vaddr, mem_res.start, resource_size(&mem_res), *(size_t *)vaddr);
+
+	*size = *(size_t *)vaddr;
+	*addr = vaddr + sizeof(size_t);
+	return 0;
+}
+
+static const struct of_device_id ima_buffer_pass_ids[] = {
+	{
+		.compatible = "linux,ima_buffer_pass",
+	},
+	{}
+};
+
+static const struct of_device_id ima_buffer_pass_match[] = {
+	{
+		.name = "ima_buffer_pass",
+	},
+};
+MODULE_DEVICE_TABLE(of, ima_buffer_pass_match);
+
+static int __init ima_buffer_pass_init(void)
+{
+	int ret = 0;
+	struct device_node *memnp;
+	struct device_node *ima_buffer_pass_node;
+
+	ima_buffer_pass_node = of_find_matching_node(NULL, ima_buffer_pass_ids);
+	if (!ima_buffer_pass_node)
+		return -ENOENT;
+
+	memnp = of_parse_phandle(ima_buffer_pass_node, "memory-region", 0);
+	if (!memnp)
+		return -ENXIO;
+
+	ret = of_address_to_resource(memnp, 0, &mem_res);
+	if (ret < 0)
+		return -ENOENT;
+
+	of_node_put(memnp);
+	dtb_status_enabled = true;
+
+	return ret;
+}
+
+static void __exit ima_buffer_pass_exit(void)
+{
+	pr_info("trying to exit the ima driver\n");
+}
+
+module_init(ima_buffer_pass_init);
+module_exit(ima_buffer_pass_exit);
diff --git a/include/linux/of.h b/include/linux/of.h
index c669c0a4732f..85ce2f24024f 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1485,4 +1485,38 @@ static inline int of_overlay_notifier_unregister(struct notifier_block *nb)
 
 #endif
 
+#ifdef CONFIG_OF_IMA
+int of_is_ima_memory_reserved(void);
+int of_remove_ima_buffer(void);
+int of_get_ima_buffer(void **addr, size_t *size);
+size_t of_ima_get_size_allocated(void);
+int of_ima_write_buffer(void *ima_buffer,
+		size_t ima_buffer_size);
+#else
+static inline int of_is_ima_memory_reserved(void)
+{
+	return -ENOTSUPP;
+};
+static inline int of_remove_ima_buffer(void)
+{
+	return -ENOTSUPP;
+};
+
+static inline int of_get_ima_buffer(void **addr, size_t *size)
+{
+	return -ENOTSUPP;
+};
+
+static inline size_t of_ima_get_size_allocated(void)
+{
+	return 0;
+};
+
+static inline int of_ima_write_buffer(void *ima_buffer,
+				      size_t ima_buffer_size)
+{
+	return -ENOTSUPP;
+};
+#endif
+
 #endif /* _LINUX_OF_H */
-- 
2.25.1


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

end of thread, other threads:[~2020-06-01  4:05 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-04 20:38 [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Prakhar Srivastava
2020-05-04 20:38 ` [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava
2020-05-12 23:09   ` Rob Herring
2020-05-18 20:34     ` Prakhar Srivastava
2020-05-04 20:38 ` [RFC][PATCH 2/2] Add support for ima buffer pass using reserved memory arm64 Prakhar Srivastava
2020-05-05  9:59 ` [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass Mark Rutland
2020-05-07  5:50   ` Prakhar Srivastava
2020-05-12 23:05     ` Rob Herring
2020-05-18 20:16       ` Prakhar Srivastava
2020-05-23  4:08         ` Thiago Jung Bauermann
2020-06-01  4:05           ` Prakhar Srivastava
  -- strict thread matches above, loose matches on Subject: below --
2020-05-01 18:15 Prakhar Srivastava
2020-05-01 18:15 ` [RFC]{PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree " Prakhar Srivastava

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