On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

This is a preperation to add eventlog support described in TCG PC Client PFP spec.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

lib/efi_loader/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index b2ab48a048..a87bf3cc98 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE int "EFI_TCG2_PROTOCOL EventLog size" depends on EFI_TCG2_PROTOCOL

• default 4096
  

• default 16384
  

I found this text in EDK II:

Minimum length(in bytes) of the system preboot TCG event log area(LAML)

For PC Client Implementation spec up to and including 1.2 the minimum log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Thanks, Masahisa Kojima

Why should ours be smaller?

Best regards

Heinrich

...

  help
          Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that
          this is going to be allocated twice. One for the eventlog it self

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote:

Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

This is a preperation to add eventlog support described in TCG PC Client PFP spec.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

lib/efi_loader/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index b2ab48a048..a87bf3cc98 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE int "EFI_TCG2_PROTOCOL EventLog size" depends on EFI_TCG2_PROTOCOL

• default 4096
  

• default 16384
  

I found this text in EDK II:

Minimum length(in bytes) of the system preboot TCG event log area(LAML)

For PC Client Implementation spec up to and including 1.2 the minimum log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we put this in the bloblist? We want to avoid adding code in EFI which is in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot. Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

Thanks, Masahisa Kojima

• Simon

...

Thanks, Masahisa Kojima

...

Why should ours be smaller?

Best regards

Heinrich

...

  help
          Define the size of the EventLog for EFI_TCG2_PROTOCOL. Note that
          this is going to be allocated twice. One for the eventlog it self

Hi Ilias,

On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote:

...

Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

This is a preperation to add eventlog support described in TCG PC Client PFP spec.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

lib/efi_loader/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index b2ab48a048..a87bf3cc98 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE int "EFI_TCG2_PROTOCOL EventLog size" depends on EFI_TCG2_PROTOCOL

• default 4096
  

• default 16384
  

I found this text in EDK II:

Minimum length(in bytes) of the system preboot TCG event log area(LAML)

For PC Client Implementation spec up to and including 1.2 the minimum log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we put this in the bloblist? We want to avoid adding code in EFI which is in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot. Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

Even in that case the eventlog can't be appended. The TCG eventlog hould be copied into EFI memory, since the kernel expects to find it there.

Typically bloblist is relocated by U-Boot. There are lots of tables that must be passed to linux, including ACPI and SMBIOS. With bloblist they can all be in one place.

What we could do is copy the contents of that buffer to the eventlog. Depending on what that buffer already has (e.g the starting header of the eventlog), we might need to strip it from the efi_tcg.c code.

I'm not really sure, but the eventlog is not just EFI thing, right? The code should be generic.

Regards, Simon

Hi Ilias,

On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

...

Hi Ilias,

On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote:

...

Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote: > > This is a preperation to add eventlog support > > described in TCG PC Client PFP spec. > > > > Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org > > --- > > lib/efi_loader/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index b2ab48a048..a87bf3cc98 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL > > config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE > > int "EFI_TCG2_PROTOCOL EventLog size" > > depends on EFI_TCG2_PROTOCOL > > - default 4096 > > + default 16384 > > I found this text in EDK II: > > Minimum length(in bytes) of the system preboot TCG event log area(LAML) > ----------------------------------------------------------------------- > > For PC Client Implementation spec up to and including 1.2 the minimum > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we put this in the bloblist? We want to avoid adding code in EFI which is in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot. Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

Even in that case the eventlog can't be appended. The TCG eventlog hould be copied into EFI memory, since the kernel expects to find it there.

Typically bloblist is relocated by U-Boot. There are lots of tables that must be passed to linux, including ACPI and SMBIOS. With bloblist they can all be in one place.

The eventlog must be allocated in EFI memory though.

There is really only one memory in U-Boot. I feel that all stuff that EFI passes on to linux should be in a single bloblist.

...

...

What we could do is copy the contents of that buffer to the eventlog. Depending on what that buffer already has (e.g the starting header of the eventlog), we might need to strip it from the efi_tcg.c code.

I'm not really sure, but the eventlog is not just EFI thing, right? The code should be generic.

It's purely an EFI construct. Specifically the entire spec, and even the log format for the eventlog are described in https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specificat...

For some reason I have seen this in ACPI, or something similar. Perhaps I was getting confused.

We need to find ways to implement EFI things with generic code. I'm not 100% sure about this particular thing, but since we already create something similar with ACPI I think we should at least look at doing it in one place.

Regards, Simon

Hi Heinrich,

On Thu, 15 Jul 2021 at 08:38, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 7/15/21 2:57 PM, Simon Glass wrote:

...

Hi Ilias,

On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

...

Hi Ilias,

On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote: > > Hi Masahisa, > > On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote: >> >> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote: >>> >>> >>> >>> On 7/7/21 3:36 PM, Masahisa Kojima wrote: >>>> This is a preperation to add eventlog support >>>> described in TCG PC Client PFP spec. >>>> >>>> Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org >>>> --- >>>> lib/efi_loader/Kconfig | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >>>> index b2ab48a048..a87bf3cc98 100644 >>>> --- a/lib/efi_loader/Kconfig >>>> +++ b/lib/efi_loader/Kconfig >>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL >>>> config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE >>>> int "EFI_TCG2_PROTOCOL EventLog size" >>>> depends on EFI_TCG2_PROTOCOL >>>> - default 4096 >>>> + default 16384 >>> >>> I found this text in EDK II: >>> >>> Minimum length(in bytes) of the system preboot TCG event log area(LAML) >>> ----------------------------------------------------------------------- >>> >>> For PC Client Implementation spec up to and including 1.2 the minimum >>> log size is 64KB. (SecurityPkg/SecurityPkg.dec) >> >> Thank you for your feedback. >> I have not checked this. >> TCG spec also says "The Log Area Minimum Length for the TCG event log >> MUST be at least 64KB." in ACPI chapter. >> I will update to set 64KB as default. >> > > Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we > put this in the bloblist? We want to avoid adding code in EFI which is > in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot. Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

Even in that case the eventlog can't be appended. The TCG eventlog hould be copied into EFI memory, since the kernel expects to find it there.

Typically bloblist is relocated by U-Boot. There are lots of tables that must be passed to linux, including ACPI and SMBIOS. With bloblist they can all be in one place.

The eventlog must be allocated in EFI memory though.

There is really only one memory in U-Boot. I feel that all stuff that EFI passes on to linux should be in a single bloblist.

We have should follow existing standards and not invent our own. LInux is not the only OS booted via U-Boot.

Perhaps we can talk about it in the next call. My point is not about avoiding standards!

What I am saying is that if we put things in a bloblist, and make that available to Linux (or other OS) via EFI, things should work, but non-EFI people are happy too. See the ACPI stuff for example - we put all of those bits in a bloblist, which is really just a contiguous area of memory. It is more convenient for U-Boot than allocating memory willy nilly. Plus the 'bloblist' command lets you see what is there.

Anyway I really don't understand all of this well enough to say what we should do. I am just passing on hints. There is way too much 'separate' EFI code in U-Boot at present and we need to work to reduce that and hopefully not add more.

[..]

Regards, Simon

Hi Heinrich,

On Thu, 15 Jul 2021 at 09:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 15.07.21 17:18, Simon Glass wrote:

...

Hi Heinrich,

On Thu, 15 Jul 2021 at 08:38, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/15/21 2:57 PM, Simon Glass wrote:

...

Hi Ilias,

On Thu, 15 Jul 2021 at 00:20, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

On Wed, Jul 14, 2021 at 08:52:07AM -0600, Simon Glass wrote:

...

Hi Ilias,

On Mon, 12 Jul 2021 at 03:28, Ilias Apalodimas ilias.apalodimas@linaro.org wrote: > > On Mon, 12 Jul 2021 at 11:40, Masahisa Kojima > masahisa.kojima@linaro.org wrote: >> >> Hi Simon, >> >> On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote: >>> >>> Hi Masahisa, >>> >>> On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote: >>>> >>>> On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote: >>>>> >>>>> >>>>> >>>>> On 7/7/21 3:36 PM, Masahisa Kojima wrote: >>>>>> This is a preperation to add eventlog support >>>>>> described in TCG PC Client PFP spec. >>>>>> >>>>>> Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org >>>>>> --- >>>>>> lib/efi_loader/Kconfig | 2 +- >>>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig >>>>>> index b2ab48a048..a87bf3cc98 100644 >>>>>> --- a/lib/efi_loader/Kconfig >>>>>> +++ b/lib/efi_loader/Kconfig >>>>>> @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL >>>>>> config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE >>>>>> int "EFI_TCG2_PROTOCOL EventLog size" >>>>>> depends on EFI_TCG2_PROTOCOL >>>>>> - default 4096 >>>>>> + default 16384 >>>>> >>>>> I found this text in EDK II: >>>>> >>>>> Minimum length(in bytes) of the system preboot TCG event log area(LAML) >>>>> ----------------------------------------------------------------------- >>>>> >>>>> For PC Client Implementation spec up to and including 1.2 the minimum >>>>> log size is 64KB. (SecurityPkg/SecurityPkg.dec) >>>> >>>> Thank you for your feedback. >>>> I have not checked this. >>>> TCG spec also says "The Log Area Minimum Length for the TCG event log >>>> MUST be at least 64KB." in ACPI chapter. >>>> I will update to set 64KB as default. >>>> >>> >>> Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we >>> put this in the bloblist? We want to avoid adding code in EFI which is >>> in U-Boot. >> >> I think bloblist is used for data passing from SPL/TPL to u-boot. >> Is your comment saying that the eventlog generated >> in u-boot(done in efi_tcg2.c with this patch series) should be appended >> into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob? >> > > Even in that case the eventlog can't be appended. The TCG eventlog > hould be copied into EFI memory, since the kernel expects to find it > there.

Typically bloblist is relocated by U-Boot. There are lots of tables that must be passed to linux, including ACPI and SMBIOS. With bloblist they can all be in one place.

The eventlog must be allocated in EFI memory though.

There is really only one memory in U-Boot. I feel that all stuff that EFI passes on to linux should be in a single bloblist.

We have should follow existing standards and not invent our own. LInux is not the only OS booted via U-Boot.

Perhaps we can talk about it in the next call. My point is not about avoiding standards!

What I am saying is that if we put things in a bloblist, and make that available to Linux (or other OS) via EFI, things should work, but

Which operating would be aware of your bloblist? Windows, BSD, Haiku?

None, it is not necessary. The bloblist is a U-Boot construct, a container for blobs. We can pass a pointer to the blob through the EFI tables, as we do with ACPI.

We want U-Boot to be interchangable with other UEFI firmware like EDK II. This will only work if we program against the same specs and don't invent new interfaces.

This is not a new interface. Let's chat about it in a contributor call.

Regards, Simon

Best regards

Heinrich

...

non-EFI people are happy too. See the ACPI stuff for example - we put all of those bits in a bloblist, which is really just a contiguous area of memory. It is more convenient for U-Boot than allocating memory willy nilly. Plus the 'bloblist' command lets you see what is there.

Anyway I really don't understand all of this well enough to say what we should do. I am just passing on hints. There is way too much 'separate' EFI code in U-Boot at present and we need to work to reduce that and hopefully not add more.

[..]

Regards, Simon

Hi Simon, Ilias,

On Wed, 14 Jul 2021 at 23:50, Simon Glass sjg@chromium.org wrote:

Hi Masahisa,

On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote:

...

Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

This is a preperation to add eventlog support described in TCG PC Client PFP spec.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

lib/efi_loader/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index b2ab48a048..a87bf3cc98 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE int "EFI_TCG2_PROTOCOL EventLog size" depends on EFI_TCG2_PROTOCOL

• default 4096
  

• default 16384
  

I found this text in EDK II:

Minimum length(in bytes) of the system preboot TCG event log area(LAML)

For PC Client Implementation spec up to and including 1.2 the minimum log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we put this in the bloblist? We want to avoid adding code in EFI which is in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot.

It can also be used to place things in memory that end up accessed by Linux, e.g. ACPI tables. So I think it fits.

I understand bloblist can be used for eventlog, I will check further.

...

Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

I suppose so, but this logic should be implemented in the TPM layer I think, not just in EFI. Otherwise we end up with another parallel implementation.

Current u-boot/lib/efi_loader/efi_tcg2.c includes the code not directly related to EFI. I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c into two files.

1) u-boot/lib/efi_loader/efi_tcg2.c This file implements the EFI interfaces required in TCG EFI Protocol Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).

2) u-boot/lib/tcg2.c(new file) This file implements the functionality required in TCG PC Client Platform Firmware Profile Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmw...). This file contains the common functions to extend eventlog and PCRs, etc.

In addition, this is different topic, I found some tpm related files under u-boot/lib directory, I think it better to have new directory like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c and tpm-common.c into lib/tcg(new directory).

Thanks, Masahisa Kojima

Regards, Simon

On Thu, 15 Jul 2021 at 15:46, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

On Thu, Jul 15, 2021 at 02:09:57PM +0900, Masahisa Kojima wrote:

...

Hi Simon, Ilias,

On Wed, 14 Jul 2021 at 23:50, Simon Glass sjg@chromium.org wrote:

...

Hi Masahisa,

On Mon, 12 Jul 2021 at 02:40, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

Hi Simon,

On Sun, 11 Jul 2021 at 09:01, Simon Glass sjg@chromium.org wrote:

...

Hi Masahisa,

On Wed, 7 Jul 2021 at 20:21, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Wed, 7 Jul 2021 at 22:47, Heinrich Schuchardt xypron.glpk@gmx.de wrote: > > > > On 7/7/21 3:36 PM, Masahisa Kojima wrote: > > This is a preperation to add eventlog support > > described in TCG PC Client PFP spec. > > > > Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org > > --- > > lib/efi_loader/Kconfig | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > index b2ab48a048..a87bf3cc98 100644 > > --- a/lib/efi_loader/Kconfig > > +++ b/lib/efi_loader/Kconfig > > @@ -327,7 +327,7 @@ config EFI_TCG2_PROTOCOL > > config EFI_TCG2_PROTOCOL_EVENTLOG_SIZE > > int "EFI_TCG2_PROTOCOL EventLog size" > > depends on EFI_TCG2_PROTOCOL > > - default 4096 > > + default 16384 > > I found this text in EDK II: > > Minimum length(in bytes) of the system preboot TCG event log area(LAML) > ----------------------------------------------------------------------- > > For PC Client Implementation spec up to and including 1.2 the minimum > log size is 64KB. (SecurityPkg/SecurityPkg.dec)

Thank you for your feedback. I have not checked this. TCG spec also says "The Log Area Minimum Length for the TCG event log MUST be at least 64KB." in ACPI chapter. I will update to set 64KB as default.

Is this the same as the BLOBLISTT_TPM2_TCG_LOG thing? If so, can we put this in the bloblist? We want to avoid adding code in EFI which is in U-Boot.

I think bloblist is used for data passing from SPL/TPL to u-boot.

It can also be used to place things in memory that end up accessed by Linux, e.g. ACPI tables. So I think it fits.

I understand bloblist can be used for eventlog, I will check further.

...

...

Is your comment saying that the eventlog generated in u-boot(done in efi_tcg2.c with this patch series) should be appended into the buffer pointed by BLOBLISTT_TPM2_TCG_LOG blob?

I suppose so, but this logic should be implemented in the TPM layer I think, not just in EFI. Otherwise we end up with another parallel implementation.

Current u-boot/lib/efi_loader/efi_tcg2.c includes the code not directly related to EFI. I would like to suggest to divide u-boot/lib/efi_loader/efi_tcg2.c into two files.

1. u-boot/lib/efi_loader/efi_tcg2.c

This file implements the EFI interfaces required in TCG EFI Protocol Specification(https://trustedcomputinggroup.org/resource/tcg-efi-protocol-specification/).

The only problem I see with the way the efi tcg2 eventlog is currently created, is that it's all done during the efi init. Ideally this should happen earlier, especially if SPL or TF-A create their own eventlog.

The status with TF-A atm is that it only creates an eventlog which then copies to secure and non-secure memory, but it doesnt extend the PCRs. We should pick that eventlog from u-boot (or better op-tee), extend the PCRs based on the information of the log and then use it as our basis and start appending events there.

...

2. u-boot/lib/tcg2.c(new file)

This file implements the functionality required in TCG PC Client Platform Firmware Profile Specification(https://trustedcomputinggroup.org/resource/pc-client-specific-platform-firmw...). This file contains the common functions to extend eventlog and PCRs, etc.

Splitting up the pc client spec bits is probably a good idea. What do you have in mind? Moving tcg2_pcr_extend() and tcg2_agile_log_append() as well, or just the pc client related wrappers?

Sorry but I was confused. I checked spec again, there are many duplication in TCG EFI Protocol spec and TCG PC Client PFP spec. For example, tdTCG_EfiSpecIdEvent structure is defined in both spec. On second thought, it is difficult to split the pc client spec into new file, so I would like to withdraw my suggestion earlier.

Thanks, Masahisa Kojima

...

In addition, this is different topic, I found some tpm related files under u-boot/lib directory, I think it better to have new directory like "tcg" and move tpm related files such as tpm_api.c, tpm-v2.c and tpm-common.c into lib/tcg(new directory).

+1

Regards /Ilias

...

Thanks, Masahisa Kojima

...

Regards, Simon

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

TCG PC Client PFP spec requires to measure the secure boot policy before validating the UEFI image. This commit adds the secure boot variable measurement of "SecureBoot", "PK", "KEK", "db" and "dbx".

Note that this implementation assumes that secure boot variables are pre-configured and not be set/updated in runtime.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

include/efi_tcg2.h | 20 ++++++ lib/efi_loader/efi_tcg2.c | 135 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+)

diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h index bcfb98168a..8d7b77c087 100644 --- a/include/efi_tcg2.h +++ b/include/efi_tcg2.h @@ -142,6 +142,26 @@ struct efi_tcg2_final_events_table { struct tcg_pcr_event2 event[]; };

+/**

• • struct tdUEFI_VARIABLE_DATA

Please, add a reference to the "TCG PC Client PlatformFirmware Profile specification".

• • @variable_name: The vendorGUID parameter in the

• • 		GetVariable() API.
    

• • @unicode_name_length: The length in CHAR16 of the Unicode name of

• • 		the variable.
    

Where is this specified as CHAR16 count? I could not find it in the "TCG PC Client PlatformFirmware Profile specification".

• • @variable_data_length: The size of the variable data.
• • @unicode_name: The CHAR16 unicode name of the variable

• • 		without NULL-terminator.
    

• • @variable_data: The data parameter of the efi variable

• • 		in the GetVariable() API.
    

• */

+struct efi_tcg2_uefi_variable_data {

• efi_guid_t variable_name;
• u64 unicode_name_length;
• u64 variable_data_length;
• u16 unicode_name[1];
• u8 variable_data[1];

+};

• struct efi_tcg2_protocol { efi_status_t (EFIAPI * get_capability)(struct efi_tcg2_protocol *this, struct efi_tcg2_boot_service_capability *capability);

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 1319a8b378..2a248bd62a 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -78,6 +78,19 @@ static const struct digest_info hash_algo_list[] = { }, };

+struct variable_info {

• u16 *name;
• const efi_guid_t *guid;

+};

+static struct variable_info secure_variables[] = {

• {L"SecureBoot", &efi_global_variable_guid},
• {L"PK", &efi_global_variable_guid},
• {L"KEK", &efi_global_variable_guid},
• {L"db", &efi_guid_image_security_database},
• {L"dbx", &efi_guid_image_security_database},

You have to measure BootOrder and Boot#### too. See TCG PC Client Platform Firmware Profile Specification, March 30, 2016.

+};

• #define MAX_HASH_COUNT ARRAY_SIZE(hash_algo_list)

  /**

@@ -1264,6 +1277,39 @@ free_pool: return ret; }

+/**

• • tcg2_measure_event() - common function to add event log and extend PCR
• • @dev: TPM device
• • @pcr_index: PCR index
• • @event_type: type of event added
• • @size: event size
• • @event: event data
• • Return: status code
• */

+static efi_status_t EFIAPI +tcg2_measure_event(struct udevice *dev, u32 pcr_index, u32 event_type,

•    u32 size, u8 event[])
  

+{

• struct tpml_digest_values digest_list;
• efi_status_t ret;
• ret = tcg2_create_digest(event, size, &digest_list);
• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_agile_log_append(pcr_index, event_type, &digest_list,

• 		    size, event);
  

+out:

• return ret;

+}

• /**
  • efi_append_scrtm_version - Append an S-CRTM EV_S_CRTM_VERSION event on the

  • 	      eventlog and extend the PCRs
    

@@ -1294,6 +1340,88 @@ out: return ret; }

+/**

• • tcg2_measure_variable() - add variable event log and extend PCR
• • @dev: TPM device
• • @pcr_index: PCR index
• • @event_type: type of event added
• • @var_name: variable name
• • @guid: guid
• • @data_size: variable data size
• • @data: variable data
• • Return: status code
• */

+static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index,

• 			  u32 event_type, u16 *var_name,
  

• 			  const efi_guid_t *guid,
  

• 			  efi_uintn_t data_size, u8 *data)
  

+{

• u32 event_size;
• efi_status_t ret;
• struct efi_tcg2_uefi_variable_data *event;
• event_size = sizeof(event->variable_name) +

•      sizeof(event->unicode_name_length) +
  

•      sizeof(event->variable_data_length) +
  

•      (u16_strlen(var_name) * sizeof(*var_name)) + data_size;
  

Please, replace by sizeof(*var_name) by sizeof(u16) to improve readability.

• event = malloc(event_size);
• if (!event)

• return EFI_OUT_OF_RESOURCES;
  

• guidcpy(&event->variable_name, guid);
• event->unicode_name_length = u16_strlen(var_name);
• event->variable_data_length = data_size;
• memcpy(event->unicode_name, var_name,

•       (event->unicode_name_length * sizeof(*event->unicode_name)));
  

sizeof(u16)

• memcpy((u16 *)event->unicode_name + event->unicode_name_length,

•       (u8 *)data, data_size);
  

memcpy() wants void *. data is already of type u8 *. This conversion can be removed.

• ret = tcg2_measure_event(dev, pcr_index, event_type, event_size,

• 		 (u8 *)event);
  

• free(event);
• return ret;

+}

+/**

• • tcg2_measure_secure_boot_variable() - measure secure boot variables
• • @dev: TPM device
• • Return: status code
• */

+static efi_status_t tcg2_measure_secure_boot_variable(struct udevice *dev) +{

• u8 *data;
• efi_uintn_t data_size;
• u32 count, i;
• efi_status_t ret;
• count = ARRAY_SIZE(secure_variables);
• for (i = 0; i < count; i++) {

• data = efi_get_var(secure_variables[i].name,
  

• 		   secure_variables[i].guid,
  

• 		   &data_size);
  

You need to check if data==NULL. There is no guarantee that the variables exist.

Best regards

Heinrich

• ret = tcg2_measure_variable(dev, 7,
  

• 			    EV_EFI_VARIABLE_DRIVER_CONFIG,
  

• 			    secure_variables[i].name,
  

• 			    secure_variables[i].guid,
  

• 			    data_size, (u8 *)data);
  

• free(data);
  

• if (ret != EFI_SUCCESS)
  

• 	goto error;
  

• }
• /*

• * TODO: add DBT and DBR measurement support when u-boot supports
  

• * these variables.
  

• */
  

+error:

• return ret;

+}

• /**
  • efi_tcg2_register() - register EFI_TCG2_PROTOCOL

@@ -1328,6 +1456,13 @@ efi_status_t efi_tcg2_register(void) tcg2_uninit(); goto fail; }

• ret = tcg2_measure_secure_boot_variable(dev);

• if (ret != EFI_SUCCESS) {

• tcg2_uninit();
  

• goto fail;
  

• }

• return ret;

  fail:

On Fri, 9 Jul 2021 at 02:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

TCG PC Client PFP spec requires to measure "Boot####" and "BootOrder" variables, EV_SEPARATOR event prior to the Ready to Boot invocation. Since u-boot does not implement Ready to Boot event, these measurements are performed when efi_start_image() is called.

TCG spec also requires to measure "Calling EFI Application from Boot Option" for each boot attempt, and "Returning from EFI Application from Boot Option" if a boot device returns control back to the Boot Manager.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

include/efi_loader.h | 4 ++ include/tpm-v2.h | 18 ++++- lib/efi_loader/efi_boottime.c | 20 ++++++ lib/efi_loader/efi_tcg2.c | 123 ++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 0a9c82a257..281ffff30f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size); efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */ void efi_variables_boot_exit_notify(void); +/* Measure efi application invocation */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void); +/* Measure efi application exit */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void); /* Called by bootefi to initialize root node */ efi_status_t efi_root_node_register(void); /* Called by bootefi to initialize runtime */ diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 3e48e35861..8a7b7f1874 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -73,7 +73,7 @@ struct udevice; /*

• event types, cf.
• "TCG PC Client Platform Firmware Profile Specification", Family "2.0"

• • rev 1.04, June 3, 2019

• • Level 00 Version 1.05 Revision 23, May 7, 2021
  */ #define EV_EFI_EVENT_BASE ((u32)0x80000000) #define EV_EFI_VARIABLE_DRIVER_CONFIG ((u32)0x80000001)

@@ -85,8 +85,24 @@ struct udevice; #define EV_EFI_ACTION ((u32)0x80000007) #define EV_EFI_PLATFORM_FIRMWARE_BLOB ((u32)0x80000008) #define EV_EFI_HANDOFF_TABLES ((u32)0x80000009) +#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x8000000A) +#define EV_EFI_HANDOFF_TABLES2 ((u32)0x8000000B) +#define EV_EFI_VARIABLE_BOOT2 ((u32)0x8000000C) #define EV_EFI_HCRTM_EVENT ((u32)0x80000010) #define EV_EFI_VARIABLE_AUTHORITY ((u32)0x800000E0) +#define EV_EFI_SPDM_FIRMWARE_BLOB ((u32)0x800000E1) +#define EV_EFI_SPDM_FIRMWARE_CONFIG ((u32)0x800000E2)

+#define EFI_CALLING_EFI_APPLICATION \

• "Calling EFI Application from Boot Option"
  

+#define EFI_RETURNING_FROM_EFI_APPLICATION \

• "Returning from EFI Application from Boot Option"
  

+#define EFI_EXIT_BOOT_SERVICES_INVOCATION \

• "Exit Boot Services Invocation"
  

+#define EFI_EXIT_BOOT_SERVICES_FAILED \

• "Exit Boot Services Returned with Failure"
  

+#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED \

• "Exit Boot Services Returned with Success"
  

Which spec defines if the string in the event log shall be utf-8 or utf-16?

TCG PC Client PFP spec does not clearly define the character encoding. In my understanding, the string derived from UEFI spec such as UEFI variable name uses utf-16(CHAR16). Other strings like "Calling EFI Application from Boot Option" defind in TCG PC Client spec use 1 byte ASCII encoding.

EDK2 implementation also uses 1 byte ASCII encoding for these strings, and tpm2-tools::tpm2_eventlog command can handles properly.

Thanks, Masahisa Kojima

Best regards

Heinrich

...

/* TPMS_TAGGED_PROPERTY Structure */ struct tpms_tagged_property { diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f6d5ba05e3..2914800c56 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, image_obj->exit_status = &exit_status; image_obj->exit_jmp = &exit_jmp;

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_invocation();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

• /* call the image! */
  if (setjmp(&exit_jmp)) {
          /*
  

@@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, exit_status != EFI_SUCCESS) efi_delete_image(image_obj, loaded_image_protocol);

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_exit();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

• /* Make sure entry/exit counts for EFI world cross-overs match */
  EFI_EXIT(exit_status);
  
  

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 2a248bd62a..6e903e3cb3 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -35,6 +35,7 @@ struct event_log_buffer { };

static struct event_log_buffer event_log; +static bool tcg2_efi_app_invoked; /*

• When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset.
• Since the current tpm2_get_capability() response buffers starts at

@@ -1383,6 +1384,128 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index, return ret; }

+/**

• • tcg2_measure_boot_variable() - measure boot variables
• • @dev: TPM device
• • Return: status code
• */

+static efi_status_t tcg2_measure_boot_variable(struct udevice *dev) +{

• u16 *boot_order;
  

• u16 var_name[] = L"BootOrder";
  

• u16 boot_name[] = L"Boot0000";
  

• u16 hexmap[] = L"0123456789ABCDEF";
  

• u8 *bootvar;
  

• efi_uintn_t var_data_size;
  

• u32 count, i;
  

• efi_status_t ret;
  

• boot_order = efi_get_var(var_name, &efi_global_variable_guid,
  

•                          &var_data_size);
  

• if (!boot_order) {
  

•         log_info("BootOrder not defined\n");
  

•         ret = EFI_NOT_FOUND;
  

•         goto error;
  

• }
  

• ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
  

•                             &efi_global_variable_guid, var_data_size,
  

•                             (u8 *)boot_order);
  

• if (ret != EFI_SUCCESS)
  

•         goto error;
  

• count = var_data_size / sizeof(*boot_order);
  

• for (i = 0; i < count; i++) {
  

•         boot_name[4] = hexmap[(boot_order[i] & 0xf000) >> 12];
  

•         boot_name[5] = hexmap[(boot_order[i] & 0x0f00) >> 8];
  

•         boot_name[6] = hexmap[(boot_order[i] & 0x00f0) >> 4];
  

•         boot_name[7] = hexmap[(boot_order[i] & 0x000f)];
  

•         bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
  

•                               &var_data_size);
  

•         if (!bootvar) {
  

•                 log_info("%ls not found\n", boot_name);
  

•                 continue;
  

•         }
  

•         ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
  

•                                     boot_name,
  

•                                     &efi_global_variable_guid,
  

•                                     var_data_size, bootvar);
  

•         free(bootvar);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto error;
  

• }
  

+error:

• free(boot_order);
  

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void) +{

• efi_status_t ret;
  

• u32 pcr_index;
  

• struct udevice *dev;
  

• u32 event = 0;
  

• if (tcg2_efi_app_invoked)
  

•         return EFI_SUCCESS;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_boot_variable(dev);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_CALLING_EFI_APPLICATION),
  

•                          (u8 *)EFI_CALLING_EFI_APPLICATION);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
  

•         ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
  

•                                  sizeof(event), (u8 *)&event);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto out;
  

• }
  

• tcg2_efi_app_invoked = true;
  

+out:

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_exit() - measure efi app exit
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void) +{

• efi_status_t ret;
  

• struct udevice *dev;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
  

•                          (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
  

• return ret;
  

+}

• /**
  • tcg2_measure_secure_boot_variable() - measure secure boot variables

On 13.07.21 10:31, Masahisa Kojima wrote:

Hi Heinrich,

...

...

...

TCG spec also requires to measure "Calling EFI Application from Boot Option" for each boot attempt, and "Returning from EFI Application from Boot Option" if a boot device returns control back to the Boot Manager.

I would like to hear your opinion regarding "Calling EFI Application from Boot Option" measurement.

My current(v1 patch series) implementation considers both "bootefi bootmgr" and "bootefi $image_addr" cases, so I do this "Calling EFI Application from Boot Option" measurement at efi_boottime.c::efi_start_image(). Do I need to implement only the case UEFI application boot from bootmgr? If yes, I will move the timing of this measurement at efi_bootmgr.c::efi_bootmgr_load().

As a reference, in edk2, this measurement is performed in ready_to_boot event handler, ready_to_boot handler is called upon the user selects the boot option in boot manager.

When booting you can call

bootefi $driver1 booefii $driver2 bootefi bootmgr

in sequence.

Any of the binaries can call LoadImage(), StartImage() multiple times to execute further images. E.g. I am loading iPXE. By default it loads GRUB from an iSCSI drive but I can choose in the menu or the iPXE console to invoke another UEFI binary.

I suggest to measure any image no matter how it is invoked. The measurement must depend on the sequence of invocation.

Best regards

Heinrich

What do you think?

Thanks, Masahisa Kojima

On Fri, 9 Jul 2021 at 11:44, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Fri, 9 Jul 2021 at 02:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

TCG PC Client PFP spec requires to measure "Boot####" and "BootOrder" variables, EV_SEPARATOR event prior to the Ready to Boot invocation. Since u-boot does not implement Ready to Boot event, these measurements are performed when efi_start_image() is called.

TCG spec also requires to measure "Calling EFI Application from Boot Option" for each boot attempt, and "Returning from EFI Application from Boot Option" if a boot device returns control back to the Boot Manager.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

include/efi_loader.h | 4 ++ include/tpm-v2.h | 18 ++++- lib/efi_loader/efi_boottime.c | 20 ++++++ lib/efi_loader/efi_tcg2.c | 123 ++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 0a9c82a257..281ffff30f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size); efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */ void efi_variables_boot_exit_notify(void); +/* Measure efi application invocation */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void); +/* Measure efi application exit */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void); /* Called by bootefi to initialize root node */ efi_status_t efi_root_node_register(void); /* Called by bootefi to initialize runtime */ diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 3e48e35861..8a7b7f1874 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -73,7 +73,7 @@ struct udevice; /* * event types, cf. * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"

• • rev 1.04, June 3, 2019

• • Level 00 Version 1.05 Revision 23, May 7, 2021 */
  #define EV_EFI_EVENT_BASE ((u32)0x80000000) #define EV_EFI_VARIABLE_DRIVER_CONFIG ((u32)0x80000001)

@@ -85,8 +85,24 @@ struct udevice; #define EV_EFI_ACTION ((u32)0x80000007) #define EV_EFI_PLATFORM_FIRMWARE_BLOB ((u32)0x80000008) #define EV_EFI_HANDOFF_TABLES ((u32)0x80000009) +#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x8000000A) +#define EV_EFI_HANDOFF_TABLES2 ((u32)0x8000000B) +#define EV_EFI_VARIABLE_BOOT2 ((u32)0x8000000C) #define EV_EFI_HCRTM_EVENT ((u32)0x80000010) #define EV_EFI_VARIABLE_AUTHORITY ((u32)0x800000E0) +#define EV_EFI_SPDM_FIRMWARE_BLOB ((u32)0x800000E1) +#define EV_EFI_SPDM_FIRMWARE_CONFIG ((u32)0x800000E2)

+#define EFI_CALLING_EFI_APPLICATION \

• "Calling EFI Application from Boot Option"
  

+#define EFI_RETURNING_FROM_EFI_APPLICATION \

• "Returning from EFI Application from Boot Option"
  

+#define EFI_EXIT_BOOT_SERVICES_INVOCATION \

• "Exit Boot Services Invocation"
  

+#define EFI_EXIT_BOOT_SERVICES_FAILED \

• "Exit Boot Services Returned with Failure"
  

+#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED \

• "Exit Boot Services Returned with Success"
  

Which spec defines if the string in the event log shall be utf-8 or utf-16?

TCG PC Client PFP spec does not clearly define the character encoding. In my understanding, the string derived from UEFI spec such as UEFI variable name uses utf-16(CHAR16). Other strings like "Calling EFI Application from Boot Option" defind in TCG PC Client spec use 1 byte ASCII encoding.

EDK2 implementation also uses 1 byte ASCII encoding for these strings, and tpm2-tools::tpm2_eventlog command can handles properly.

Thanks, Masahisa Kojima

...

Best regards

Heinrich

...

/* TPMS_TAGGED_PROPERTY Structure */ struct tpms_tagged_property { diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f6d5ba05e3..2914800c56 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, image_obj->exit_status = &exit_status; image_obj->exit_jmp = &exit_jmp;

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_invocation();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

•  /* call the image! */
   if (setjmp(&exit_jmp)) {
           /*
  

@@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, exit_status != EFI_SUCCESS) efi_delete_image(image_obj, loaded_image_protocol);

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_exit();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

•  /* Make sure entry/exit counts for EFI world cross-overs match */
   EFI_EXIT(exit_status);
  
  

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 2a248bd62a..6e903e3cb3 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -35,6 +35,7 @@ struct event_log_buffer { };

static struct event_log_buffer event_log; +static bool tcg2_efi_app_invoked; /* * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset. * Since the current tpm2_get_capability() response buffers starts at @@ -1383,6 +1384,128 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index, return ret; }

+/**

• • tcg2_measure_boot_variable() - measure boot variables
• • @dev: TPM device
• • Return: status code
• */

+static efi_status_t tcg2_measure_boot_variable(struct udevice *dev) +{

• u16 *boot_order;
  

• u16 var_name[] = L"BootOrder";
  

• u16 boot_name[] = L"Boot0000";
  

• u16 hexmap[] = L"0123456789ABCDEF";
  

• u8 *bootvar;
  

• efi_uintn_t var_data_size;
  

• u32 count, i;
  

• efi_status_t ret;
  

• boot_order = efi_get_var(var_name, &efi_global_variable_guid,
  

•                          &var_data_size);
  

• if (!boot_order) {
  

•         log_info("BootOrder not defined\n");
  

•         ret = EFI_NOT_FOUND;
  

•         goto error;
  

• }
  

• ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
  

•                             &efi_global_variable_guid, var_data_size,
  

•                             (u8 *)boot_order);
  

• if (ret != EFI_SUCCESS)
  

•         goto error;
  

• count = var_data_size / sizeof(*boot_order);
  

• for (i = 0; i < count; i++) {
  

•         boot_name[4] = hexmap[(boot_order[i] & 0xf000) >> 12];
  

•         boot_name[5] = hexmap[(boot_order[i] & 0x0f00) >> 8];
  

•         boot_name[6] = hexmap[(boot_order[i] & 0x00f0) >> 4];
  

•         boot_name[7] = hexmap[(boot_order[i] & 0x000f)];
  

•         bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
  

•                               &var_data_size);
  

•         if (!bootvar) {
  

•                 log_info("%ls not found\n", boot_name);
  

•                 continue;
  

•         }
  

•         ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
  

•                                     boot_name,
  

•                                     &efi_global_variable_guid,
  

•                                     var_data_size, bootvar);
  

•         free(bootvar);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto error;
  

• }
  

+error:

• free(boot_order);
  

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void) +{

• efi_status_t ret;
  

• u32 pcr_index;
  

• struct udevice *dev;
  

• u32 event = 0;
  

• if (tcg2_efi_app_invoked)
  

•         return EFI_SUCCESS;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_boot_variable(dev);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_CALLING_EFI_APPLICATION),
  

•                          (u8 *)EFI_CALLING_EFI_APPLICATION);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
  

•         ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
  

•                                  sizeof(event), (u8 *)&event);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto out;
  

• }
  

• tcg2_efi_app_invoked = true;
  

+out:

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_exit() - measure efi app exit
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void) +{

• efi_status_t ret;
  

• struct udevice *dev;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
  

•                          (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
  

• return ret;
  

+}

• /**
  • tcg2_measure_secure_boot_variable() - measure secure boot variables

Hi Heinrich, Akashi-san,

Thank you for your comment. I will keep current implementation.

Thanks, Masahisa Kojima

On Wed, 14 Jul 2021 at 08:54, AKASHI Takahiro takahiro.akashi@linaro.org wrote:

On Tue, Jul 13, 2021 at 04:24:52PM +0200, Heinrich Schuchardt wrote:

...

On 13.07.21 10:31, Masahisa Kojima wrote:

...

Hi Heinrich,

...

...

...

TCG spec also requires to measure "Calling EFI Application from Boot Option" for each boot attempt, and "Returning from EFI Application from Boot Option" if a boot device returns control back to the Boot Manager.

I would like to hear your opinion regarding "Calling EFI Application from Boot Option" measurement.

My current(v1 patch series) implementation considers both "bootefi bootmgr" and "bootefi $image_addr" cases, so I do this "Calling EFI Application from Boot Option" measurement at efi_boottime.c::efi_start_image(). Do I need to implement only the case UEFI application boot from bootmgr? If yes, I will move the timing of this measurement at efi_bootmgr.c::efi_bootmgr_load().

As a reference, in edk2, this measurement is performed in ready_to_boot event handler, ready_to_boot handler is called upon the user selects the boot option in boot manager.

When booting you can call

bootefi $driver1 booefii $driver2 bootefi bootmgr

in sequence.

Any of the binaries can call LoadImage(), StartImage() multiple times to execute further images. E.g. I am loading iPXE. By default it loads GRUB from an iSCSI drive but I can choose in the menu or the iPXE console to invoke another UEFI binary.

I suggest to measure any image no matter how it is invoked. The measurement must depend on the sequence of invocation.

Moreover, booting from the default path, like /EFI/BOOT/BOOTAA64.EFI, is only implemented by using bootefi <addr> syntax.

-Takahiro Akashi

...

Best regards

Heinrich

...

What do you think?

Thanks, Masahisa Kojima

On Fri, 9 Jul 2021 at 11:44, Masahisa Kojima masahisa.kojima@linaro.org wrote:

...

On Fri, 9 Jul 2021 at 02:46, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

TCG PC Client PFP spec requires to measure "Boot####" and "BootOrder" variables, EV_SEPARATOR event prior to the Ready to Boot invocation. Since u-boot does not implement Ready to Boot event, these measurements are performed when efi_start_image() is called.

TCG spec also requires to measure "Calling EFI Application from Boot Option" for each boot attempt, and "Returning from EFI Application from Boot Option" if a boot device returns control back to the Boot Manager.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

include/efi_loader.h | 4 ++ include/tpm-v2.h | 18 ++++- lib/efi_loader/efi_boottime.c | 20 ++++++ lib/efi_loader/efi_tcg2.c | 123 ++++++++++++++++++++++++++++++++++ 4 files changed, 164 insertions(+), 1 deletion(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 0a9c82a257..281ffff30f 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -407,6 +407,10 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size); efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */ void efi_variables_boot_exit_notify(void); +/* Measure efi application invocation */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void); +/* Measure efi application exit */ +efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void); /* Called by bootefi to initialize root node */ efi_status_t efi_root_node_register(void); /* Called by bootefi to initialize runtime */ diff --git a/include/tpm-v2.h b/include/tpm-v2.h index 3e48e35861..8a7b7f1874 100644 --- a/include/tpm-v2.h +++ b/include/tpm-v2.h @@ -73,7 +73,7 @@ struct udevice; /* * event types, cf. * "TCG PC Client Platform Firmware Profile Specification", Family "2.0"

• • rev 1.04, June 3, 2019

• • Level 00 Version 1.05 Revision 23, May 7, 2021 */
  #define EV_EFI_EVENT_BASE ((u32)0x80000000) #define EV_EFI_VARIABLE_DRIVER_CONFIG ((u32)0x80000001)

@@ -85,8 +85,24 @@ struct udevice; #define EV_EFI_ACTION ((u32)0x80000007) #define EV_EFI_PLATFORM_FIRMWARE_BLOB ((u32)0x80000008) #define EV_EFI_HANDOFF_TABLES ((u32)0x80000009) +#define EV_EFI_PLATFORM_FIRMWARE_BLOB2 ((u32)0x8000000A) +#define EV_EFI_HANDOFF_TABLES2 ((u32)0x8000000B) +#define EV_EFI_VARIABLE_BOOT2 ((u32)0x8000000C) #define EV_EFI_HCRTM_EVENT ((u32)0x80000010) #define EV_EFI_VARIABLE_AUTHORITY ((u32)0x800000E0) +#define EV_EFI_SPDM_FIRMWARE_BLOB ((u32)0x800000E1) +#define EV_EFI_SPDM_FIRMWARE_CONFIG ((u32)0x800000E2)

+#define EFI_CALLING_EFI_APPLICATION \

• "Calling EFI Application from Boot Option"
  

+#define EFI_RETURNING_FROM_EFI_APPLICATION \

• "Returning from EFI Application from Boot Option"
  

+#define EFI_EXIT_BOOT_SERVICES_INVOCATION \

• "Exit Boot Services Invocation"
  

+#define EFI_EXIT_BOOT_SERVICES_FAILED \

• "Exit Boot Services Returned with Failure"
  

+#define EFI_EXIT_BOOT_SERVICES_SUCCEEDED \

• "Exit Boot Services Returned with Success"
  

Which spec defines if the string in the event log shall be utf-8 or utf-16?

TCG PC Client PFP spec does not clearly define the character encoding. In my understanding, the string derived from UEFI spec such as UEFI variable name uses utf-16(CHAR16). Other strings like "Calling EFI Application from Boot Option" defind in TCG PC Client spec use 1 byte ASCII encoding.

EDK2 implementation also uses 1 byte ASCII encoding for these strings, and tpm2-tools::tpm2_eventlog command can handles properly.

Thanks, Masahisa Kojima

...

Best regards

Heinrich

...

/* TPMS_TAGGED_PROPERTY Structure */ struct tpms_tagged_property { diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index f6d5ba05e3..2914800c56 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2993,6 +2993,16 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, image_obj->exit_status = &exit_status; image_obj->exit_jmp = &exit_jmp;

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_invocation();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

•  /* call the image! */
   if (setjmp(&exit_jmp)) {
           /*
  

@@ -3251,6 +3261,16 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, exit_status != EFI_SUCCESS) efi_delete_image(image_obj, loaded_image_protocol);

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL)) {
  

•         if (image_obj->image_type == IMAGE_SUBSYSTEM_EFI_APPLICATION) {
  

•                 ret = efi_tcg2_measure_efi_app_exit();
  

•                 if (ret != EFI_SUCCESS) {
  

•                         EFI_PRINT("tcg2 measurement fails(0x%lx)\n",
  

•                                   ret);
  

•                 }
  

•         }
  

• }
  

•  /* Make sure entry/exit counts for EFI world cross-overs match */
   EFI_EXIT(exit_status);
  
  

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 2a248bd62a..6e903e3cb3 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -35,6 +35,7 @@ struct event_log_buffer { };

static struct event_log_buffer event_log; +static bool tcg2_efi_app_invoked; /* * When requesting TPM2_CAP_TPM_PROPERTIES the value is on a standard offset. * Since the current tpm2_get_capability() response buffers starts at @@ -1383,6 +1384,128 @@ static efi_status_t tcg2_measure_variable(struct udevice *dev, u32 pcr_index, return ret; }

+/**

• • tcg2_measure_boot_variable() - measure boot variables
• • @dev: TPM device
• • Return: status code
• */

+static efi_status_t tcg2_measure_boot_variable(struct udevice *dev) +{

• u16 *boot_order;
  

• u16 var_name[] = L"BootOrder";
  

• u16 boot_name[] = L"Boot0000";
  

• u16 hexmap[] = L"0123456789ABCDEF";
  

• u8 *bootvar;
  

• efi_uintn_t var_data_size;
  

• u32 count, i;
  

• efi_status_t ret;
  

• boot_order = efi_get_var(var_name, &efi_global_variable_guid,
  

•                          &var_data_size);
  

• if (!boot_order) {
  

•         log_info("BootOrder not defined\n");
  

•         ret = EFI_NOT_FOUND;
  

•         goto error;
  

• }
  

• ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2, var_name,
  

•                             &efi_global_variable_guid, var_data_size,
  

•                             (u8 *)boot_order);
  

• if (ret != EFI_SUCCESS)
  

•         goto error;
  

• count = var_data_size / sizeof(*boot_order);
  

• for (i = 0; i < count; i++) {
  

•         boot_name[4] = hexmap[(boot_order[i] & 0xf000) >> 12];
  

•         boot_name[5] = hexmap[(boot_order[i] & 0x0f00) >> 8];
  

•         boot_name[6] = hexmap[(boot_order[i] & 0x00f0) >> 4];
  

•         boot_name[7] = hexmap[(boot_order[i] & 0x000f)];
  

•         bootvar = efi_get_var(boot_name, &efi_global_variable_guid,
  

•                               &var_data_size);
  

•         if (!bootvar) {
  

•                 log_info("%ls not found\n", boot_name);
  

•                 continue;
  

•         }
  

•         ret = tcg2_measure_variable(dev, 1, EV_EFI_VARIABLE_BOOT2,
  

•                                     boot_name,
  

•                                     &efi_global_variable_guid,
  

•                                     var_data_size, bootvar);
  

•         free(bootvar);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto error;
  

• }
  

+error:

• free(boot_order);
  

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_invocation() - measure efi app invocation
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void) +{

• efi_status_t ret;
  

• u32 pcr_index;
  

• struct udevice *dev;
  

• u32 event = 0;
  

• if (tcg2_efi_app_invoked)
  

•         return EFI_SUCCESS;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_boot_variable(dev);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_CALLING_EFI_APPLICATION),
  

•                          (u8 *)EFI_CALLING_EFI_APPLICATION);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• for (pcr_index = 0; pcr_index <= 7; pcr_index++) {
  

•         ret = tcg2_measure_event(dev, pcr_index, EV_SEPARATOR,
  

•                                  sizeof(event), (u8 *)&event);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto out;
  

• }
  

• tcg2_efi_app_invoked = true;
  

+out:

• return ret;
  

+}

+/**

• • efi_tcg2_measure_efi_app_exit() - measure efi app exit
• • Return: status code
• */

+efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void) +{

• efi_status_t ret;
  

• struct udevice *dev;
  

• ret = platform_get_tpm2_device(&dev);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• ret = tcg2_measure_event(dev, 4, EV_EFI_ACTION,
  

•                          strlen(EFI_RETURNING_FROM_EFI_APPLICATION),
  

•                          (u8 *)EFI_RETURNING_FROM_EFI_APPLICATION);
  

• return ret;
  

+}

• /**
  • tcg2_measure_secure_boot_variable() - measure secure boot variables

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

TCG PC Client PFP spec requires to measure "Exit Boot Services Invocation" if ExitBootServices() is invoked. Depending upon the return code from the ExitBootServices() call, "Exit Boot Services Returned with Success" or "Exit Boot Services Returned with Failure" is also measured.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

include/efi_loader.h | 1 + lib/efi_loader/efi_boottime.c | 5 +++ lib/efi_loader/efi_tcg2.c | 70 +++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 281ffff30f..e9bd1aac08 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -407,6 +407,7 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size); efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */ void efi_variables_boot_exit_notify(void); +efi_status_t efi_tcg2_notify_exit_boot_services_failed(void); /* Measure efi application invocation */ efi_status_t EFIAPI efi_tcg2_measure_efi_app_invocation(void); /* Measure efi application exit */ diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 2914800c56..6e07ef65bc 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -2181,6 +2181,11 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle, efi_set_watchdog(0); WATCHDOG_RESET(); out:

• if (ret != EFI_SUCCESS) {

• if (IS_ENABLED(CONFIG_EFI_TCG2_PROTOCOL))
  

• 	efi_tcg2_notify_exit_boot_services_failed();
  

• }
• return EFI_EXIT(ret); }

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 6e903e3cb3..823abd8217 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1506,6 +1506,67 @@ efi_status_t EFIAPI efi_tcg2_measure_efi_app_exit(void) return ret; }

+/**

• • efi_tcg2_notify_exit_boot_services() - ExitBootService callback
• • @event: callback event
• • @context: callback context
• */

+static void EFIAPI +efi_tcg2_notify_exit_boot_services(struct efi_event *event, void *context) +{

• efi_status_t ret;
• struct udevice *dev;
• EFI_ENTRY("%p, %p", event, context);
• ret = platform_get_tpm2_device(&dev);
• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,

• 		 strlen(EFI_EXIT_BOOT_SERVICES_INVOCATION),
  

• 		 (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,

• 		 strlen(EFI_EXIT_BOOT_SERVICES_SUCCEEDED),
  

• 		 (u8 *)EFI_EXIT_BOOT_SERVICES_SUCCEEDED);
  

+out:

• EFI_EXIT(ret);

+}

+/**

• • efi_tcg2_notify_exit_boot_services_failed()
• • • notify ExitBootServices() is failed
• • Return: status code
• */

+efi_status_t efi_tcg2_notify_exit_boot_services_failed(void) +{

• struct udevice *dev;
• efi_status_t ret;
• ret = platform_get_tpm2_device(&dev);
• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,

• 		 sizeof(EFI_EXIT_BOOT_SERVICES_INVOCATION),
  

• 		 (u8 *)EFI_EXIT_BOOT_SERVICES_INVOCATION);
  

• if (ret != EFI_SUCCESS)

• goto out;
  

• ret = tcg2_measure_event(dev, 5, EV_EFI_ACTION,

• 		 sizeof(EFI_EXIT_BOOT_SERVICES_FAILED),
  

• 		 (u8 *)EFI_EXIT_BOOT_SERVICES_FAILED);
  

+out:

• return ret;

+}

We must keep out code small. Please, carve out a function to which pass a the event type and an u8 string and use it where applicable.

Best regards

Heinrich

• /**
  • tcg2_measure_secure_boot_variable() - measure secure boot variables

@@ -1556,6 +1617,7 @@ efi_status_t efi_tcg2_register(void) { efi_status_t ret = EFI_SUCCESS; struct udevice *dev;

• struct efi_event *event;

  ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) {

@@ -1580,6 +1642,14 @@ efi_status_t efi_tcg2_register(void) goto fail; }

• ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK,

• 	       efi_tcg2_notify_exit_boot_services, NULL,
  

• 	       NULL, &event);
  

• if (ret != EFI_SUCCESS) {

• tcg2_uninit();
  

• goto fail;
  

• }
• ret = tcg2_measure_secure_boot_variable(dev); if (ret != EFI_SUCCESS) { tcg2_uninit();

On Fri, 9 Jul 2021 at 02:32, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 7/7/21 3:36 PM, Masahisa Kojima wrote:

...

Refactor efi_append_scrtm_version() to use common function for adding eventlog and extending PCR.

Signed-off-by: Masahisa Kojima masahisa.kojima@linaro.org

lib/efi_loader/efi_tcg2.c | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 823abd8217..00e442cea5 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -1321,23 +1321,11 @@ out: */ static efi_status_t efi_append_scrtm_version(struct udevice *dev) {

• struct tpml_digest_values digest_list;
  u8 ver[] = U_BOOT_VERSION_STRING;
  

• const int pcr_index = 0;
  efi_status_t ret;
  
  

• ret = tcg2_create_digest(ver, sizeof(ver), &digest_list);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• ret = tcg2_measure_event(dev, 0, EV_S_CRTM_VERSION, sizeof(ver), ver);
  

Must we convert the string to UTF-16? What is required to get a correct listing of the event in the OS?

TCG PC Client spec just says "The event field contains the version string of the SRTM.". I think there is no character encoding requirement.

Thanks, Masahisa Kojima

Best regards

Heinrich

...

• ret = tcg2_pcr_extend(dev, pcr_index, &digest_list);
  

• if (ret != EFI_SUCCESS)
  

•         goto out;
  

• ret = tcg2_agile_log_append(pcr_index, EV_S_CRTM_VERSION, &digest_list,
  

•                             sizeof(ver), ver);
  

-out: return ret; }