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

[Eddie James <eajames@linux.ibm.com>]

On 1/4/23 03:56, Etienne Carriere wrote:
> Hello Eddie and all,
>
> On Tue, 3 Jan 2023 at 21:42, Eddie James <eajames@linux.ibm.com> wrote:
>> Add a configuration option to measure the boot through the bootm
>> function.
>>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   boot/bootm.c    | 53 +++++++++++++++++++++++++++++++++++++++++++++++++
>>   cmd/bootm.c     |  2 ++
>>   common/Kconfig  |  6 ++++++
>>   include/image.h |  1 +
>>   4 files changed, 62 insertions(+)
>>
>> diff --git a/boot/bootm.c b/boot/bootm.c
>> index a4c0870c0f..7f64d79035 100644
>> --- a/boot/bootm.c
>> +++ b/boot/bootm.c
>> @@ -22,6 +22,9 @@
>>   #include <asm/global_data.h>
>>   #include <asm/io.h>
>>   #include <linux/sizes.h>
>> +#if defined(CONFIG_MEASURED_BOOT)
>> +#include <tpm-v2.h>
>> +#endif
>>   #if defined(CONFIG_CMD_USB)
>>   #include <usb.h>
>>   #endif
>> @@ -713,6 +716,56 @@ 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 defined(CONFIG_MEASURED_BOOT)
>> +       if (!ret && (states & BOOTM_STATE_MEASURE)) {
>> +               void *initrd_buf;
>> +               void *image_buf;
>> +               const char *s;
>> +               u32 rd_len;
>> +
>> +               ret = tcg2_measurement_init();
>> +               if (ret)
>> +                       goto measure_err;
>> +
>> +               image_buf = map_sysmem(images->os.image_start,
>> +                                      images->os.image_len);
>> +               ret = tcg2_measure_data(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(8, rd_len, initrd_buf, EV_COMPACT_HASH,
>> +                                       strlen("initrd") + 1, (u8 *)"initrd");
>> +               if (ret)
>> +                       goto unmap_initrd;
>> +
>> +               ret = tcg2_measure_data(9, images->ft_len,
>> +                                       (u8 *)images->ft_addr,
>> +                                       EV_TABLE_OF_DEVICES, strlen("dts") + 1,
>> +                                       (u8 *)"dts");
> This unconditionally measures DTB content but maybe there are
> information that may differ at each boot as a random MAC address or an
> ASLR seed.
> Do you think something should condition measurement of the passed DTB
> as proposed using a config switch in [1]?
>
> [1] https://lore.kernel.org/all/20221207151110.529106-1-etienne.carriere@linaro.org/


That would make sense, I am happy to add that in v2. Should I use that 
config switch as-is or add a new one for the bootm measurement?

Thanks

Eddie


>
> Regards,
> Etienne
>
>> +               if (ret)
>> +                       goto unmap_initrd;
>> +
>> +               s = env_get("bootargs");
>> +               if (!s)
>> +                       s = "";
>> +               tcg2_measure_data(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();
>> +measure_err:
>> +               ret = 0;
>> +       }
>> +#endif
>> +
>>          /* Load the OS */
>>          if (!ret && (states & BOOTM_STATE_LOADOS)) {
>>                  iflag = bootm_disable_interrupts();
>> diff --git a/cmd/bootm.c b/cmd/bootm.c
>> index 37c2af96e0..0c4a713e02 100644
>> --- a/cmd/bootm.c
>> +++ b/cmd/bootm.c
>> @@ -161,6 +161,8 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
>>                  BOOTM_STATE_OS_GO;
>>          if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
>>                  states |= BOOTM_STATE_RAMDISK;
>> +       if (IS_ENABLED(CONFIG_MEASURED_BOOT))
>> +               states |= BOOTM_STATE_MEASURE;
>>          if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS))
>>                  states |= BOOTM_STATE_OS_CMDLINE;
>>          ret = do_bootm_states(cmdtp, flag, argc, argv, states, &images, 1);
>> diff --git a/common/Kconfig b/common/Kconfig
>> index 21434c5cf1..57ef68e4f3 100644
>> --- a/common/Kconfig
>> +++ b/common/Kconfig
>> @@ -799,6 +799,12 @@ config AVB_BUF_SIZE
>>
>>   endif # AVB_VERIFY
>>
>> +config MEASURED_BOOT
>> +       bool "Measure the boot to TPM and event log"
>> +       depends on HASH && TPM_V2
>> +       help
>> +         This option enables measurement of the boot process.
>> +
>>   config SCP03
>>          bool "Build SCP03 - Secure Channel Protocol O3 - controls"
>>          depends on OPTEE || SANDBOX
>> diff --git a/include/image.h b/include/image.h
>> index 6f21dafba8..b00803eeac 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -406,6 +406,7 @@ struct bootm_headers {
>>   #define BOOTM_STATE_OS_FAKE_GO 0x00000200      /* 'Almost' run the OS */
>>   #define BOOTM_STATE_OS_GO      0x00000400
>>   #define BOOTM_STATE_PRE_LOAD   0x00000800
>> +#define BOOTM_STATE_MEASURE    0x00001000
>>          int             state;
>>
>>   #if defined(CONFIG_LMB) && !defined(USE_HOSTCC)
>> --
>> 2.31.1
>>