On Fri, 16 Sept 2022 at 10:15, Ilias Apalodimas
<ilias.apalodimas@xxxxxxxxxx> wrote:
>
> The EFI TCG spec, in §10.2.6 Measuring UEFI Variables and UEFI GPT Data,
> is measuring the entire UEFI_LOAD_OPTION (in PCR5). As a result boot
> variables that point to the same UEFI application but with different
> optional data, will have distinct measurements.
>
That is not the main problem. The main problem is that
LoadImage()/StartImage() may be used to invoke things beyond Boot####
options, and at StartImage() time, the load options could be anything.
So not measuring the load options when the image is actually being
invoked is a huge oversight.
> However, PCR5 is used for more than that and there might be a need to use
> a PCR with a more limited scope which measures our initramfs and
> LoadOptions.
>
> So add a measurement in PCR9 (which we already use for our initrd) and
> extend it with the LoadOption measurements
>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@xxxxxxxxxx>
> ---
> .../firmware/efi/libstub/efi-stub-helper.c | 21 +++++++++++++++++++
> drivers/firmware/efi/libstub/efistub.h | 1 +
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
> index 3ef4867344b9..5b03248527c6 100644
> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
> @@ -370,6 +370,27 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len)
> int options_bytes = 0, safe_options_bytes = 0; /* UTF-8 bytes */
> bool in_quote = false;
> efi_status_t status;
> + static const struct efi_measured_event load_options_tcg2_event = {
> + {
> + sizeof(load_options_tcg2_event) + sizeof("Load Options"),
> + {
> + sizeof(load_options_tcg2_event.event_data.event_header),
> + EFI_TCG2_EVENT_HEADER_VERSION,
> + 9,
> + EV_EVENT_TAG,
> + },
> + },
> + {
> + LOAD_OPTIONS_EVENT_TAG_ID,
> + sizeof("Load Options"),
> + },
> + { "Load Options" },
> + };
> +
> + if (options_chars > 0)
> + efi_measure_tagged_event((unsigned long) options,
> + (unsigned long) options_chars,
> + &load_options_tcg2_event);
>
The name 'options_chars' is a bit misleading here, as it is actually
the size in bytes at this point.
> efi_apply_loadoptions_quirk((const void **)&options, &options_chars);
> options_chars /= sizeof(*options);
> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
> index cb7eb5ed9f14..e3605b383964 100644
> --- a/drivers/firmware/efi/libstub/efistub.h
> +++ b/drivers/firmware/efi/libstub/efistub.h
> @@ -741,6 +741,7 @@ union apple_properties_protocol {
> typedef u32 efi_tcg2_event_log_format;
>
> #define INITRD_EVENT_TAG_ID 0x8F3B22ECU
> +#define LOAD_OPTIONS_EVENT_TAG_ID 0x8F3B22EDU
Is this an arbitrarily chosen value?
> #define EV_EVENT_TAG 0x00000006U
> #define EFI_TCG2_EVENT_HEADER_VERSION 0x1
>
> --
> 2.34.1
>
