Re: [V2 PATCH 2/3] dt-bindings: chosen: Document ima-kexec-buffer

2020-07-13 Thread Prakhar Srivastava




On 6/19/20 5:41 PM, Thiago Jung Bauermann wrote:


Prakhar Srivastava  writes:


Integrity measurement architecture(IMA) validates if files
have been accidentally or maliciously altered, both remotely and
locally, appraise a file's measurement against a "good" value stored
as an extended attribute, and enforce local file integrity.

IMA also measures singatures of kernel and initrd during kexec along with
the command line used for kexec.
These measurements are critical to verify the seccurity posture of the OS.

Resering memory and adding the memory information to a device tree node
acts as the mechanism to carry over IMA measurement logs.

Update devicetree documentation to reflect the addition of new property
under the chosen node.


Thank you for writing this documentation patch. It's something I should
have done when I added the powerpc IMA kexec support.

You addressed Rob Herring's comments regarding the commit message, but
not the ones regarding the patch contents.

When posting a new version of the patches, make sure to address all
comments made so far. Addressing a comment doesn't necessarily mean
implementing the requested change. If you don't then you should at least
explain why you chose a different path.

I mention it because this has occurred before with this patch series,
and it's hard to make forward progress if review comments get ignored.


---
  Documentation/devicetree/bindings/chosen.txt | 17 +
  1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt 
b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..a15f70c007ef 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -135,3 +135,20 @@ e.g.
linux,initrd-end = <0x8280>;
};
  };
+
+linux,ima-kexec-buffer
+--
+
+This property(currently used by powerpc, arm64) holds the memory range,


space before the parenthesis.


+the address and the size, of the IMA measurement logs that are being carried


Maybe it's because English isn't my first language, but IMHO it's
clearer if "the address and the size" is between parentheses rather than
commas.


+over to the kexec session.


I don't think there's a "kexec session", but I'm not sure what a good
term would be. "linux,booted-from-kexec" uses "new kernel" so perhaps
that's a good option to use instead of "kexec session".


+
+/ {
+   chosen {
+   linux,ima-kexec-buffer = <0x9 0x8200 0x0 0x8000>;
+   };
+};
+
+This porperty does not represent real hardware, but the memory allocated for
+carrying the IMA measurement logs. The address and the suze are expressed in
+#address-cells and #size-cells, respectively of the root node.



I will update the descriptions and ack the comments/changes in the 
patches as well.


Thankyou,
Prakhar Srivastava

--
Thiago Jung Bauermann
IBM Linux Technology Center



Re: [V2 PATCH 1/3] Refactoring powerpc code for carrying over IMA measurement logs, to move non architecture specific code to security/ima.

2020-07-13 Thread Prakhar Srivastava




On 6/19/20 5:19 PM, Thiago Jung Bauermann wrote:


Prakhar Srivastava  writes:


Powerpc has support to carry over the IMA measurement logs. Refatoring the
non-architecture specific code out of arch/powerpc and into security/ima.

The code adds support for reserving and freeing up of memory for IMA measurement
logs.


Last week, Mimi provided this feedback:

"From your patch description, this patch should be broken up.  Moving
the non-architecture specific code out of powerpc should be one patch.
  Additional support should be in another patch.  After each patch, the
code should work properly."

That's not what you do here. You move the code, but you also make other
changes at the same time. This has two problems:

1. It makes the patch harder to review, because it's very easy to miss a
change.

2. If in the future a git bisect later points to this patch, it's not
clear whether the problem is because of the code movement, or because
of the other changes.

When you move code, ideally the patch should only make the changes
necessary to make the code work at its new location. The patch which
does code movement should not cause any change in behavior.

Other changes should go in separate patches, either before or after the
one moving the code.

More comments below.


Hi Thiago,

Apologies for the delayed response i was away for a few days.
I am working on breaking up the changes so that its easier to review and 
update as well.


Thanks,
Prakhar Srivastava



---
  arch/powerpc/include/asm/ima.h |  10 ---
  arch/powerpc/kexec/ima.c   | 126 ++---
  security/integrity/ima/ima_kexec.c | 116 ++
  3 files changed, 124 insertions(+), 128 deletions(-)

diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
index ead488cf3981..c29ec86498f8 100644
--- a/arch/powerpc/include/asm/ima.h
+++ b/arch/powerpc/include/asm/ima.h
@@ -4,15 +4,6 @@

  struct kimage;

-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
-
  #ifdef CONFIG_IMA_KEXEC
  int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
  size_t size);
@@ -22,7 +13,6 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, 
int chosen_node);
  static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
   int chosen_node)
  {
-   remove_ima_buffer(fdt, chosen_node);
return 0;
  }


This is wrong. Even if the currently running kernel doesn't have
CONFIG_IMA_KEXEC, it should remove the IMA buffer property and memory
reservation from the FDT that is being prepared for the next kernel.

This is because the IMA kexec buffer is useless for the next kernel,
regardless of whether the current kernel supports CONFIG_IMA_KEXEC or
not. Keeping it around would be a waste of memory.


I will keep it in my next revision.
My understanding was the reserved memory is freed and property removed 
when IMA loads the logs on init. During setup_fdt in kexec, a duplicate 
copy of the dt is used, but memory still needs to be allocated, thus the 
property itself indicats presence of reserved memory.




@@ -179,13 +64,18 @@ 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);


This is wrong, for the same reason stated above.


if (!image->arch.ima_buffer_size)
return 0;

-   ret = get_addr_size_cells(&addr_cells, &size_cells);
-   if (ret)
+   ret = fdt_address_cells(fdt, chosen_node);
+   if (ret < 0)
+   return ret;
+   addr_cells = ret;
+
+   ret = fdt_size_cells(fdt, chosen_node);
+   if (ret < 0)
return ret;
+   size_cells = ret;

entry_size = 4 * (addr_cells + size_cells);



I liked this change. Thanks! I agree it's better to use
fdt_address_cells() and fdt_size_cells() here.

But it should be in a separate patch. Either before or after the one
moving the code.


diff --git a/security/integrity/ima/ima_kexec.c 
b/security/integrity/ima/ima_kexec.c
index 121de3e04af2..e1e6d6154015 100644
--- a/security/integrity/ima/ima_kexec.c
+++ b/security/integrity/ima/ima_kexec.c
@@ -10,8 +10,124 @@
  #include 
  #include 
  #include 
+#include 
+#include 
+#include 
  #include "ima.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_p

[V2 PATCH 3/3] Add support for arm64 to carry over IMA measurement logs

2020-06-18 Thread Prakhar Srivastava
Add support for arm64 to carry over IMA measurement logs.
Update arm64 code to call into functions made available in patch 1/3.

---
 arch/arm64/Kconfig |  1 +
 arch/arm64/include/asm/ima.h   | 17 ++
 arch/arm64/include/asm/kexec.h |  3 ++
 arch/arm64/kernel/machine_kexec_file.c | 47 +-
 4 files changed, 60 insertions(+), 8 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5d513f461957..3d544e2e25e6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1070,6 +1070,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 ..70ac39b74607
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARCH_IMA_H
+#define _ASM_ARCH_IMA_H
+
+struct kimage;
+
+#ifdef CONFIG_IMA_KEXEC
+int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
+ size_t size);
+#else
+static inline int arch_ima_add_kexec_buffer(struct kimage *image,
+   unsigned long load_addr, size_t size)
+{
+   return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARCH_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d24b527e8c00..7bd60c185ad3 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -100,6 +100,9 @@ struct kimage_arch {
void *elf_headers;
unsigned long elf_headers_mem;
unsigned long elf_headers_sz;
+
+   phys_addr_t ima_buffer_addr;
+   size_t ima_buffer_size;
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/machine_kexec_file.c 
b/arch/arm64/kernel/machine_kexec_file.c
index b40c3b0def92..1e9007c926db 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -24,20 +24,37 @@
 #include 
 
 /* relevant device tree properties */
-#define FDT_PROP_KEXEC_ELFHDR  "linux,elfcorehdr"
-#define FDT_PROP_MEM_RANGE "linux,usable-memory-range"
-#define FDT_PROP_INITRD_START  "linux,initrd-start"
-#define FDT_PROP_INITRD_END"linux,initrd-end"
-#define FDT_PROP_BOOTARGS  "bootargs"
-#define FDT_PROP_KASLR_SEED"kaslr-seed"
-#define FDT_PROP_RNG_SEED  "rng-seed"
-#define RNG_SEED_SIZE  128
+#define FDT_PROP_KEXEC_ELFHDR  "linux,elfcorehdr"
+#define FDT_PROP_MEM_RANGE "linux,usable-memory-range"
+#define FDT_PROP_INITRD_START  "linux,initrd-start"
+#define FDT_PROP_INITRD_END"linux,initrd-end"
+#define FDT_PROP_BOOTARGS  "bootargs"
+#define FDT_PROP_KASLR_SEED"kaslr-seed"
+#define FDT_PROP_RNG_SEED  "rng-seed"
+#define FDT_PROP_IMA_KEXEC_BUFFER  "linux,ima-kexec-buffer"
+#define RNG_SEED_SIZE  128
 
 const struct kexec_file_ops * const kexec_file_loaders[] = {
&kexec_image_ops,
NULL
 };
 
+/**
+ * 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;
+}
+
 int arch_kimage_file_post_load_cleanup(struct kimage *image)
 {
vfree(image->arch.dtb);
@@ -66,6 +83,9 @@ static int setup_dtb(struct kimage *image,
if (ret && ret != -FDT_ERR_NOTFOUND)
goto out;
ret = fdt_delprop(dtb, off, FDT_PROP_MEM_RANGE);
+   if (ret && ret != -FDT_ERR_NOTFOUND)
+   goto out;
+   ret = fdt_delprop(dtb, off, FDT_PROP_IMA_KEXEC_BUFFER);
if (ret && ret != -FDT_ERR_NOTFOUND)
goto out;
 
@@ -119,6 +139,17 @@ static int setup_dtb(struct kimage *image,
goto out;
}
 
+   if (image->arch.ima_buffer_size > 0) {
+
+   ret = fdt_appendprop_addrrange(dtb, 0, off,
+   FDT_PROP_IMA_KEXEC_BUFFER,
+   image->arch.ima_buffer_addr,
+   image->arch.ima_buffer_size);
+   if (ret)
+   return (ret == -FDT_ERR_NOSPACE ? -ENOMEM : -EINVAL);
+
+   }
+
/* add kaslr-seed */
ret = fdt_delprop(dtb, off, FDT_PROP_KASLR_SEED);
if (ret == -FDT_ERR_NOTFOUND)
-- 
2.25.1



[V2 PATCH 2/3] dt-bindings: chosen: Document ima-kexec-buffer

2020-06-18 Thread Prakhar Srivastava
Integrity measurement architecture(IMA) validates if files
have been accidentally or maliciously altered, both remotely and
locally, appraise a file's measurement against a "good" value stored
as an extended attribute, and enforce local file integrity.

IMA also measures singatures of kernel and initrd during kexec along with
the command line used for kexec.
These measurements are critical to verify the seccurity posture of the OS.

Resering memory and adding the memory information to a device tree node
acts as the mechanism to carry over IMA measurement logs.

Update devicetree documentation to reflect the addition of new property
under the chosen node. 

---
 Documentation/devicetree/bindings/chosen.txt | 17 +
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt 
b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..a15f70c007ef 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -135,3 +135,20 @@ e.g.
linux,initrd-end = <0x8280>;
};
 };
+
+linux,ima-kexec-buffer
+--
+
+This property(currently used by powerpc, arm64) holds the memory range,
+the address and the size, of the IMA measurement logs that are being carried
+over to the kexec session.
+
+/ {
+   chosen {
+   linux,ima-kexec-buffer = <0x9 0x8200 0x0 0x8000>;
+   };
+};
+
+This porperty does not represent real hardware, but the memory allocated for
+carrying the IMA measurement logs. The address and the suze are expressed in
+#address-cells and #size-cells, respectively of the root node.
-- 
2.25.1



[V2 PATCH 0/3] Adding support for carrying IMA measurement logs

2020-06-18 Thread Prakhar Srivastava
Integrgity Measurement Architecture(IMA) during kexec(kexec file load)
verifies the kernel signature and measures the signature of the kernel.

The signature in the measuremnt logs is used to verfiy the 
authenticity of the kernel in the subsequent kexec'd session, however in
the current implementation IMA measurement logs are not carried over thus
remote attesation cannot verify the signature of the running kernel.

Adding support to arm64 to carry over the IMA measurement logs over kexec.

Add a new chosen node entry linux,ima-kexec-buffer to hold the address and
the size of the memory reserved to carry the IMA measurement log.
Refactor existing powerpc code to be used by amr64 as well.  

Changelog:

v2:
  Break patches into separate patches.
  - Powerpc related Refactoring
  - Updating the docuemntation for chosen node
  - Updating arm64 to support IMA buffer pass

v1:
  Refactoring carrying over IMA measuremnet logs over Kexec. This patch
moves the non-architecture specific code out of powerpc and adds to
security/ima.(Suggested by Thiago)
  Add Documentation regarding the ima-kexec-buffer node in the chosen
node documentation

v0:
  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.

Prakhar Srivastava (3):
  Refactoring powerpc code for carrying over IMA measurement logs, to
move non architecture specific code to security/ima.
  dt-bindings: chosen: Document ima-kexec-buffer carrying over IMA
measuremnt logs over kexec.
  Add support for arm64 to carry over IMA measurement logs

 Documentation/devicetree/bindings/chosen.txt |  17 +++
 arch/arm64/Kconfig   |   1 +
 arch/arm64/include/asm/ima.h |  17 +++
 arch/arm64/include/asm/kexec.h   |   3 +
 arch/arm64/kernel/machine_kexec_file.c   |  47 +--
 arch/powerpc/include/asm/ima.h   |  10 --
 arch/powerpc/kexec/ima.c | 126 ++-
 security/integrity/ima/ima_kexec.c   | 116 +
 8 files changed, 201 insertions(+), 136 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h

-- 
2.25.1



[V2 PATCH 1/3] Refactoring powerpc code for carrying over IMA measurement logs, to move non architecture specific code to security/ima.

2020-06-18 Thread Prakhar Srivastava
Powerpc has support to carry over the IMA measurement logs. Refatoring the 
non-architecture specific code out of arch/powerpc and into security/ima.

The code adds support for reserving and freeing up of memory for IMA measurement
logs.

---
 arch/powerpc/include/asm/ima.h |  10 ---
 arch/powerpc/kexec/ima.c   | 126 ++---
 security/integrity/ima/ima_kexec.c | 116 ++
 3 files changed, 124 insertions(+), 128 deletions(-)

diff --git a/arch/powerpc/include/asm/ima.h b/arch/powerpc/include/asm/ima.h
index ead488cf3981..c29ec86498f8 100644
--- a/arch/powerpc/include/asm/ima.h
+++ b/arch/powerpc/include/asm/ima.h
@@ -4,15 +4,6 @@
 
 struct kimage;
 
-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
-
 #ifdef CONFIG_IMA_KEXEC
 int arch_ima_add_kexec_buffer(struct kimage *image, unsigned long load_addr,
  size_t size);
@@ -22,7 +13,6 @@ int setup_ima_buffer(const struct kimage *image, void *fdt, 
int chosen_node);
 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 */
diff --git a/arch/powerpc/kexec/ima.c b/arch/powerpc/kexec/ima.c
index 720e50e490b6..6054ce91d2a6 100644
--- a/arch/powerpc/kexec/ima.c
+++ b/arch/powerpc/kexec/ima.c
@@ -12,121 +12,6 @@
 #include 
 #include 
 
-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");
-}
-
 #ifdef CONFIG_IMA_KEXEC
 /**
  * arch_ima_add_kexec_buffer - do arch-specific steps to add the IMA buffer
@@ -179,13 +64,18 @@ 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 (!imag

[v1 PATCH 2/2] Add Documentation regarding the ima-kexec-buffer node in the chosen node documentation

2020-06-07 Thread Prakhar Srivastava
Add Documentation regarding the ima-kexec-buffer node in
 the chosen node documentation
 
Signed-off-by: Prakhar Srivastava 
---
 Documentation/devicetree/bindings/chosen.txt | 17 +
 1 file changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/chosen.txt 
b/Documentation/devicetree/bindings/chosen.txt
index 45e79172a646..a15f70c007ef 100644
--- a/Documentation/devicetree/bindings/chosen.txt
+++ b/Documentation/devicetree/bindings/chosen.txt
@@ -135,3 +135,20 @@ e.g.
linux,initrd-end = <0x8280>;
};
 };
+
+linux,ima-kexec-buffer
+--
+
+This property(currently used by powerpc, arm64) holds the memory range,
+the address and the size, of the IMA measurement logs that are being carried
+over to the kexec session.
+
+/ {
+   chosen {
+   linux,ima-kexec-buffer = <0x9 0x8200 0x0 0x8000>;
+   };
+};
+
+This porperty does not represent real hardware, but the memory allocated for
+carrying the IMA measurement logs. The address and the suze are expressed in
+#address-cells and #size-cells, respectively of the root node.
-- 
2.25.1



[v1 PATCH 0/2] Adding support to carry IMA measurement logs

2020-06-07 Thread Prakhar Srivastava
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.

Add a new chosen node entry linux,ima-kexec-buffer to hold the address and
the size of the memory reserved to carry the IMA measurement log.

Tested on:
  arm64 with Uboot

Changelog:

v1:
  Refactoring carrying over IMA measuremnet logs over Kexec. This patch
moves the non-architecture specific code out of powerpc and adds to
security/ima.(Suggested by Thiago)
  Add Documentation regarding the ima-kexec-buffer node in the chosen
node documentation

v0:
  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.

 Documentation/devicetree/bindings/chosen.txt |  17 +++
 arch/arm64/Kconfig   |   1 +
 arch/arm64/include/asm/ima.h |  24 +++
 arch/arm64/include/asm/kexec.h   |   3 +
 arch/arm64/kernel/machine_kexec_file.c   |  47 +-
 arch/powerpc/include/asm/ima.h   |   9 --
 arch/powerpc/kexec/ima.c | 117 +-
 security/integrity/ima/ima_kexec.c   | 151 +++
 8 files changed, 236 insertions(+), 133 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h

-- 
2.25.1



[v1 PATCH 1/2] Refactoring carrying over IMA measuremnet logs over Kexec.

2020-06-07 Thread Prakhar Srivastava
This patch moves the non-architecture specific code out of powerpc and
 adds to security/ima. 
Update the arm64 and powerpc kexec file load paths to carry the IMA measurement
logs.

Signed-off-by: Prakhar Srivastava 
---
 arch/arm64/Kconfig |   1 +
 arch/arm64/include/asm/ima.h   |  24 
 arch/arm64/include/asm/kexec.h |   3 +
 arch/arm64/kernel/machine_kexec_file.c |  47 ++--
 arch/powerpc/include/asm/ima.h |   9 --
 arch/powerpc/kexec/ima.c   | 117 +--
 security/integrity/ima/ima_kexec.c | 151 +
 7 files changed, 219 insertions(+), 133 deletions(-)
 create mode 100644 arch/arm64/include/asm/ima.h

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 5d513f461957..3d544e2e25e6 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1070,6 +1070,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 ..8946bae8baa2
--- /dev/null
+++ b/arch/arm64/include/asm/ima.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_ARCH_IMA_H
+#define _ASM_ARCH_IMA_H
+
+struct kimage;
+
+#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 arch_ima_add_kexec_buffer(struct kimage *image,
+   unsigned long load_addr, size_t size)
+{
+   return 0;
+}
+static inline int setup_ima_buffer(const struct kimage *image, void *fdt,
+  int chosen_node)
+{
+   return 0;
+}
+#endif /* CONFIG_IMA_KEXEC */
+#endif /* _ASM_ARCH_IMA_H */
diff --git a/arch/arm64/include/asm/kexec.h b/arch/arm64/include/asm/kexec.h
index d24b527e8c00..7bd60c185ad3 100644
--- a/arch/arm64/include/asm/kexec.h
+++ b/arch/arm64/include/asm/kexec.h
@@ -100,6 +100,9 @@ struct kimage_arch {
void *elf_headers;
unsigned long elf_headers_mem;
unsigned long elf_headers_sz;
+
+   phys_addr_t ima_buffer_addr;
+   size_t ima_buffer_size;
 };
 
 extern const struct kexec_file_ops kexec_image_ops;
diff --git a/arch/arm64/kernel/machine_kexec_file.c 
b/arch/arm64/kernel/machine_kexec_file.c
index b40c3b0def92..1e9007c926db 100644
--- a/arch/arm64/kernel/machine_kexec_file.c
+++ b/arch/arm64/kernel/machine_kexec_file.c
@@ -24,20 +24,37 @@
 #include 
 
 /* relevant device tree properties */
-#define FDT_PROP_KEXEC_ELFHDR  "linux,elfcorehdr"
-#define FDT_PROP_MEM_RANGE "linux,usable-memory-range"
-#define FDT_PROP_INITRD_START  "linux,initrd-start"
-#define FDT_PROP_INITRD_END"linux,initrd-end"
-#define FDT_PROP_BOOTARGS  "bootargs"
-#define FDT_PROP_KASLR_SEED"kaslr-seed"
-#define FDT_PROP_RNG_SEED  "rng-seed"
-#define RNG_SEED_SIZE  128
+#define FDT_PROP_KEXEC_ELFHDR  "linux,elfcorehdr"
+#define FDT_PROP_MEM_RANGE "linux,usable-memory-range"
+#define FDT_PROP_INITRD_START  "linux,initrd-start"
+#define FDT_PROP_INITRD_END"linux,initrd-end"
+#define FDT_PROP_BOOTARGS  "bootargs"
+#define FDT_PROP_KASLR_SEED"kaslr-seed"
+#define FDT_PROP_RNG_SEED  "rng-seed"
+#define FDT_PROP_IMA_KEXEC_BUFFER  "linux,ima-kexec-buffer"
+#define RNG_SEED_SIZE  128
 
 const struct kexec_file_ops * const kexec_file_loaders[] = {
&kexec_image_ops,
NULL
 };
 
+/**
+ * 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;
+}
+
 int arch_kimage_file_post_load_cleanup(struct kimage *image)
 {
vfree(image->arch.dtb);
@@ -66,6 +83,9 @@ static int setup_dtb(struct kimage *image,
if (ret && ret != -FDT_ERR_NOTFOUND)
goto out;
ret = fdt_delprop(dtb, off, FDT_PROP_MEM_RANGE);
+   if (ret && ret != -FDT_ERR_NOTFOUND)
+   goto out;
+   ret = fdt_delprop(dtb, off, FDT_PROP_IMA_KEXEC_BUFFER);
if (ret && ret != -FDT_ERR_NOTFOUND)
 

Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass

2020-05-31 Thread Prakhar Srivastava



On 5/22/20 9:08 PM, Thiago Jung Bauermann wrote:


Hello Prakhar,

Prakhar Srivastava  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-00c5cb6f1...@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



Re: [RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.

2020-05-18 Thread Prakhar Srivastava




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


Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass

2020-05-18 Thread Prakhar Srivastava




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-00c5cb6f1...@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



Re: [RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass

2020-05-06 Thread Prakhar Srivastava

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-00c5cb6f1...@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



[RFC][PATCH 2/2] Add support for ima buffer pass using reserved memory arm64

2020-05-04 Thread Prakhar Srivastava
 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 
---
 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 ..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 ..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 
+ */
+
+#include 
+#include 
+
+
+/**
+ * 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

[RFC][PATCH 0/2] Add support for using reserved memory for ima buffer pass

2020-05-04 Thread Prakhar Srivastava
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



[RFC][PATCH 1/2] Add a layer of abstraction to use the memory reserved by device tree for ima buffer pass.

2020-05-04 Thread Prakhar Srivastava
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 
---
 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 ..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 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+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",
+   },
+};
+