diff mbox series

tpm: measure DTB in PCR1 instead of PCR0

Message ID 20240614120951.16158-1-ilias.apalodimas@linaro.org
State Accepted
Commit d69759aec28ec195bc20e31b5558f99a32e3ab28
Headers show
Series tpm: measure DTB in PCR1 instead of PCR0 | expand

Commit Message

Ilias Apalodimas June 14, 2024, 12:09 p.m. UTC
The PC client spec [0], doesn't describe measurements for DTBs. It does
describe what do to for ACPI tables though.

There is a description for ACPI in 3.3.4.1 PCR[0] – SRTM, POST BIOS,
and Embedded Drivers and they explicitly mention ACPI in there. There's
no mention of ACPI in 3.3.4.2 PCR[1] – Host Platform Configuration.

However, in Figure 6 --  PCR Mapping of UEFI Components ACPI is shown
in PCR1. The general description also mentions PCR0 is for code and PCR1
is for data such as ACPI and SMBIOS.

So let's switch over the DTB measurements to PCR1 which seems a better
fit.

[0] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification

Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
 boot/bootm.c              | 2 +-
 lib/efi_loader/efi_tcg2.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
2.45.1

Comments

Eddie James June 14, 2024, 9:31 p.m. UTC | #1
On 6/14/24 07:09, Ilias Apalodimas wrote:
> The PC client spec [0], doesn't describe measurements for DTBs. It does
> describe what do to for ACPI tables though.
>
> There is a description for ACPI in 3.3.4.1 PCR[0] – SRTM, POST BIOS,
> and Embedded Drivers and they explicitly mention ACPI in there. There's
> no mention of ACPI in 3.3.4.2 PCR[1] – Host Platform Configuration.
>
> However, in Figure 6 --  PCR Mapping of UEFI Components ACPI is shown
> in PCR1. The general description also mentions PCR0 is for code and PCR1
> is for data such as ACPI and SMBIOS.


Thanks, looks correct.

Reviewed-by: Eddie James <eajames@linux.ibm.com>


>
> So let's switch over the DTB measurements to PCR1 which seems a better
> fit.
>
> [0] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification
>
> Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> ---
>   boot/bootm.c              | 2 +-
>   lib/efi_loader/efi_tcg2.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/boot/bootm.c b/boot/bootm.c
> index 6fa8edab021e..3de87eb185d7 100644
> --- a/boot/bootm.c
> +++ b/boot/bootm.c
> @@ -963,7 +963,7 @@ int bootm_measure(struct bootm_headers *images)
>   			goto unmap_initrd;
>
>   		if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
> -			ret = tcg2_measure_data(dev, &elog, 0, images->ft_len,
> +			ret = tcg2_measure_data(dev, &elog, 1, images->ft_len,
>   						(u8 *)images->ft_addr,
>   						EV_TABLE_OF_DEVICES,
>   						strlen("dts") + 1,
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index 51264c1b998c..a8a54c9f131d 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -1328,7 +1328,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
>   	sha256_update(&hash_ctx, (u8 *)dtb + fdt_off_mem_rsvmap(dtb), rsvmap_size);
>   	sha256_finish(&hash_ctx, blob->data + blob->blob_description_size);
>
> -	ret = measure_event(dev, 0, EV_POST_CODE, event_size, (u8 *)blob);
> +	ret = measure_event(dev, 1, EV_POST_CODE, event_size, (u8 *)blob);
>
>   	free(blob);
>   	return ret;
> --
> 2.45.1
>
Ilias Apalodimas June 15, 2024, 6:57 a.m. UTC | #2
Thanks Eddie,

On Sat, 15 Jun 2024 at 00:31, Eddie James <eajames@linux.ibm.com> wrote:
>
>
> On 6/14/24 07:09, Ilias Apalodimas wrote:
> > The PC client spec [0], doesn't describe measurements for DTBs. It does
> > describe what do to for ACPI tables though.
> >
> > There is a description for ACPI in 3.3.4.1 PCR[0] – SRTM, POST BIOS,
> > and Embedded Drivers and they explicitly mention ACPI in there. There's
> > no mention of ACPI in 3.3.4.2 PCR[1] – Host Platform Configuration.
> >
> > However, in Figure 6 --  PCR Mapping of UEFI Components ACPI is shown
> > in PCR1. The general description also mentions PCR0 is for code and PCR1
> > is for data such as ACPI and SMBIOS.
>
>
> Thanks, looks correct.
>
> Reviewed-by: Eddie James <eajames@linux.ibm.com>

Heinrich, do you want to carry this on the EFI tree, or shall I send a
PR via the TPM tree?

Thanks
/Ilias
>
>
> >
> > So let's switch over the DTB measurements to PCR1 which seems a better
> > fit.
> >
> > [0] https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmware-profile-specification
> >
> > Reported-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > ---
> >   boot/bootm.c              | 2 +-
> >   lib/efi_loader/efi_tcg2.c | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/boot/bootm.c b/boot/bootm.c
> > index 6fa8edab021e..3de87eb185d7 100644
> > --- a/boot/bootm.c
> > +++ b/boot/bootm.c
> > @@ -963,7 +963,7 @@ int bootm_measure(struct bootm_headers *images)
> >                       goto unmap_initrd;
> >
> >               if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
> > -                     ret = tcg2_measure_data(dev, &elog, 0, images->ft_len,
> > +                     ret = tcg2_measure_data(dev, &elog, 1, images->ft_len,
> >                                               (u8 *)images->ft_addr,
> >                                               EV_TABLE_OF_DEVICES,
> >                                               strlen("dts") + 1,
> > diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> > index 51264c1b998c..a8a54c9f131d 100644
> > --- a/lib/efi_loader/efi_tcg2.c
> > +++ b/lib/efi_loader/efi_tcg2.c
> > @@ -1328,7 +1328,7 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb)
> >       sha256_update(&hash_ctx, (u8 *)dtb + fdt_off_mem_rsvmap(dtb), rsvmap_size);
> >       sha256_finish(&hash_ctx, blob->data + blob->blob_description_size);
> >
> > -     ret = measure_event(dev, 0, EV_POST_CODE, event_size, (u8 *)blob);
> > +     ret = measure_event(dev, 1, EV_POST_CODE, event_size, (u8 *)blob);
> >
> >       free(blob);
> >       return ret;
> > --
> > 2.45.1
> >
diff mbox series

Patch

diff --git a/boot/bootm.c b/boot/bootm.c
index 6fa8edab021e..3de87eb185d7 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -963,7 +963,7 @@  int bootm_measure(struct bootm_headers *images)
 			goto unmap_initrd;

 		if (IS_ENABLED(CONFIG_MEASURE_DEVICETREE)) {
-			ret = tcg2_measure_data(dev, &elog, 0, images->ft_len,
+			ret = tcg2_measure_data(dev, &elog, 1, images->ft_len,
 						(u8 *)images->ft_addr,
 						EV_TABLE_OF_DEVICES,
 						strlen("dts") + 1,
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 51264c1b998c..a8a54c9f131d 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -1328,7 +1328,7 @@  efi_status_t efi_tcg2_measure_dtb(void *dtb)
 	sha256_update(&hash_ctx, (u8 *)dtb + fdt_off_mem_rsvmap(dtb), rsvmap_size);
 	sha256_finish(&hash_ctx, blob->data + blob->blob_description_size);

-	ret = measure_event(dev, 0, EV_POST_CODE, event_size, (u8 *)blob);
+	ret = measure_event(dev, 1, EV_POST_CODE, event_size, (u8 *)blob);

 	free(blob);
 	return ret;