Re: [PATCH v3 3/6] bootm: Support boot measurement

2023-01-12 Thread Simon Glass
Hi Eddie,

On Thu, 12 Jan 2023 at 09:16, Eddie James  wrote:
>
> Add a configuration option to measure the boot through the bootm
> function. Add the measurement state to the booti and bootz paths
> as well.
>
> Signed-off-by: Eddie James 
> ---
>  boot/Kconfig| 23 
>  boot/bootm.c| 70 +
>  cmd/booti.c |  1 +
>  cmd/bootm.c |  2 ++
>  cmd/bootz.c |  1 +
>  include/bootm.h |  2 ++
>  include/image.h |  1 +
>  7 files changed, 100 insertions(+)
>
> diff --git a/boot/Kconfig b/boot/Kconfig
> index 30bc182fcd..e90d61412e 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -587,6 +587,29 @@ config LEGACY_IMAGE_FORMAT
>   loaded. If a board needs the legacy image format support in this
>   case, enable it here.
>
> +config MEASURED_BOOT
> +   bool "Measure boot images and configuration to TPM and event log"
> +   depends on HASH && TPM_V2
> +   help
> + This option enables measurement of the boot process. Measurement
> + involves creating cryptographic hashes of the binary images that
> + are booting and storing them in the TPM. In addition, a log of
> + these hashes is stored in memory for the OS to verify the booted
> + images and configuration. Enable this if the OS has configured
> + some memory area for the event log and you intend to use some
> + attestation tools on your system.
> +
> +if MEASURED_BOOT
> +   config MEASURE_DEVICETREE
> +   bool "Measure the devicetree image"
> +   default y if MEASURED_BOOT
> +   help
> + On some platforms, the devicetree is not static as it may contain
> + random MAC addresses or other such data that changes each boot.
> + Therefore, it should not be measured into the TPM. In that case,
> + disable the measurement here.
> +endif # MEASURED_BOOT
> +
>  config SUPPORT_RAW_INITRD
> bool "Enable raw initrd images"
> help
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 15fce8ad95..c8423f2c60 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -22,6 +22,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #if defined(CONFIG_CMD_USB)
>  #include 
>  #endif
> @@ -659,6 +660,72 @@ int bootm_process_cmdline_env(int flags)
> return 0;
>  }
>
> +int bootm_measure(struct bootm_headers *images)
> +{
> +   int ret = 0;
> +
> +   /* Skip measurement if EFI is going to do it */
> +   if (images->os.os == IH_OS_EFI &&
> +   IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL) &&
> +   IS_ENABLED(CONFIG_BOOTM_EFI))
> +   return ret;
> +
> +   if (IS_ENABLED(CONFIG_MEASURED_BOOT)) {

Isn't this an EFI feature? I would expect MEASURED_BOOT to be a
generic U-Boot feature that can be used by non-EFI code.

So I believe what we need here is some sort of API for measuring
things, enabled by MEASURED_BOOT, with EFI providing its
implementation (in lib/efi_loader/). Driver model would be a good
approach I think, or perhaps a linker list as is done with the
environment / partitions?

In any case, adding EFI code into bootm is a non-starter.

The first thing is to define what the API is, e.g. with a header file
called measured_boot.h or similar.

> +   struct tcg2_event_log elog;
> +   struct udevice *dev;
> +   void *initrd_buf;
> +   void *image_buf;
> +   const char *s;
> +   u32 rd_len;
> +
> +   ret = tcg2_measurement_init(, );
> +   if (ret)
> +   return ret;
> +
> +   image_buf = map_sysmem(images->os.image_start,
> +  images->os.image_len);
> +   ret = tcg2_measure_data(dev, , 8, images->os.image_len,
> +   image_buf, EV_COMPACT_HASH,
> +   strlen("linux") + 1, (u8 *)"linux");
> +   if (ret)
> +   goto unmap_image;
> +
> +   rd_len = images->rd_end - images->rd_start;
> +   initrd_buf = map_sysmem(images->rd_start, rd_len);
> +   ret = tcg2_measure_data(dev, , 8, rd_len, initrd_buf,
> +   EV_COMPACT_HASH, strlen("initrd") + 1,
> +   (u8 *)"initrd");
> +   if (ret)
> +   goto unmap_initrd;
> +
> +   if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
> +   ret = tcg2_measure_data(dev, , 9, images->ft_len,
> +   (u8 *)images->ft_addr,
> +   EV_TABLE_OF_DEVICES,
> +   strlen("dts") + 1,
> +   (u8 *)"dts");
> +   if (ret)
> +   goto unmap_initrd;
> + 

[PATCH v3 3/6] bootm: Support boot measurement

2023-01-12 Thread Eddie James
Add a configuration option to measure the boot through the bootm
function. Add the measurement state to the booti and bootz paths
as well.

Signed-off-by: Eddie James 
---
 boot/Kconfig| 23 
 boot/bootm.c| 70 +
 cmd/booti.c |  1 +
 cmd/bootm.c |  2 ++
 cmd/bootz.c |  1 +
 include/bootm.h |  2 ++
 include/image.h |  1 +
 7 files changed, 100 insertions(+)

diff --git a/boot/Kconfig b/boot/Kconfig
index 30bc182fcd..e90d61412e 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -587,6 +587,29 @@ config LEGACY_IMAGE_FORMAT
  loaded. If a board needs the legacy image format support in this
  case, enable it here.
 
+config MEASURED_BOOT
+   bool "Measure boot images and configuration to TPM and event log"
+   depends on HASH && TPM_V2
+   help
+ This option enables measurement of the boot process. Measurement
+ involves creating cryptographic hashes of the binary images that
+ are booting and storing them in the TPM. In addition, a log of
+ these hashes is stored in memory for the OS to verify the booted
+ images and configuration. Enable this if the OS has configured
+ some memory area for the event log and you intend to use some
+ attestation tools on your system.
+
+if MEASURED_BOOT
+   config MEASURE_DEVICETREE
+   bool "Measure the devicetree image"
+   default y if MEASURED_BOOT
+   help
+ On some platforms, the devicetree is not static as it may contain
+ random MAC addresses or other such data that changes each boot.
+ Therefore, it should not be measured into the TPM. In that case,
+ disable the measurement here.
+endif # MEASURED_BOOT
+
 config SUPPORT_RAW_INITRD
bool "Enable raw initrd images"
help
diff --git a/boot/bootm.c b/boot/bootm.c
index 15fce8ad95..c8423f2c60 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -22,6 +22,7 @@
 #include 
 #include 
 #include 
+#include 
 #if defined(CONFIG_CMD_USB)
 #include 
 #endif
@@ -659,6 +660,72 @@ int bootm_process_cmdline_env(int flags)
return 0;
 }
 
+int bootm_measure(struct bootm_headers *images)
+{
+   int ret = 0;
+
+   /* Skip measurement if EFI is going to do it */
+   if (images->os.os == IH_OS_EFI &&
+   IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL) &&
+   IS_ENABLED(CONFIG_BOOTM_EFI))
+   return ret;
+
+   if (IS_ENABLED(CONFIG_MEASURED_BOOT)) {
+   struct tcg2_event_log elog;
+   struct udevice *dev;
+   void *initrd_buf;
+   void *image_buf;
+   const char *s;
+   u32 rd_len;
+
+   ret = tcg2_measurement_init(, );
+   if (ret)
+   return ret;
+
+   image_buf = map_sysmem(images->os.image_start,
+  images->os.image_len);
+   ret = tcg2_measure_data(dev, , 8, images->os.image_len,
+   image_buf, EV_COMPACT_HASH,
+   strlen("linux") + 1, (u8 *)"linux");
+   if (ret)
+   goto unmap_image;
+
+   rd_len = images->rd_end - images->rd_start;
+   initrd_buf = map_sysmem(images->rd_start, rd_len);
+   ret = tcg2_measure_data(dev, , 8, rd_len, initrd_buf,
+   EV_COMPACT_HASH, strlen("initrd") + 1,
+   (u8 *)"initrd");
+   if (ret)
+   goto unmap_initrd;
+
+   if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
+   ret = tcg2_measure_data(dev, , 9, images->ft_len,
+   (u8 *)images->ft_addr,
+   EV_TABLE_OF_DEVICES,
+   strlen("dts") + 1,
+   (u8 *)"dts");
+   if (ret)
+   goto unmap_initrd;
+   }
+
+   s = env_get("bootargs");
+   if (!s)
+   s = "";
+   ret = tcg2_measure_data(dev, , 1, strlen(s) + 1, (u8 *)s,
+   EV_PLATFORM_CONFIG_FLAGS,
+   strlen(s) + 1, (u8 *)s);
+
+unmap_initrd:
+   unmap_sysmem(initrd_buf);
+
+unmap_image:
+   unmap_sysmem(image_buf);
+   tcg2_measurement_term(dev, , ret != 0);
+   }
+
+   return ret;
+}
+
 /**
  * Execute selected states of the bootm command.
  *
@@ -710,6 +777,9 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int 
argc,
if (!ret && (states & BOOTM_STATE_FINDOTHER))
ret = bootm_find_other(cmdtp, flag, argc, argv);
 
+   if (IS_ENABLED(CONFIG_MEASURED_BOOT) && !ret && (states &