[PATCH] efi_loader: explicitly return EFI_UNSUPPORTED for TCG 1.0 compatibility

[Ilias Apalodimas]

Hi Stuart,


[...]

> >>     EFI_TCG2_EVENT_ALGORITHM_BITMAP ActivePcrBanks
> >> };
> >>
> >> The intent in the TCG spec is for the caller to be able to
> >> determine the size of the struct by passing in a small
> >> buffer (e.g. 1 byte buffer), and then the firmware
> >> should return sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY) in the
> >> 1-byte "Size" field.  And the return value should be
> >> EFI_BUFFER_TOO_SMALL as per the spec.
> >>
> >> In order to detect a 1.0 client, which you don't want to support,
> >> I think you need a _new_ check that is something like this:
> >>
> >> // detect 1.0 client
> >> if (capability->size < sizeof(*capability) &&
> >>       capability->size >= offsetof(EFI_TCG2_BOOT_SERVICE_CAPABILITY,
> >> NumberOfPcrBanks)) {
> >>       efi_ret = EFI_UNSUPPORTED;
> >>       goto out;
> >> }
> >>
> >> The last field in the 1.0 struct was ManufacturerID, so you should
> >> be able to detect 1.0 clients that expect that based on the size they
> >> pass in.
> >
> > The patch doesn't show the entire u-boot code there and is a bit misleading.
> > There's another check that precedes the one I am patching, which if I
> > am reading it correctly does exactly what you describe
> >
> > #define BOOT_SERVICE_CAPABILITY_MIN \
> >            offsetof(struct efi_tcg2_boot_service_capability, number_of_pcr_banks)
> > if (capability->size < BOOT_SERVICE_CAPABILITY_MIN) {
> >            capability->size = BOOT_SERVICE_CAPABILITY_MIN;
> >            efi_ret = EFI_BUFFER_TOO_SMALL;
> >            goto out;
> > }
>
> I took a look at the u-boot code.  What I think we want is this:
>
> diff --git a/include/efi_tcg2.h b/include/efi_tcg2.h
> index b1c3abd097..6a46d9e51c 100644
> --- a/include/efi_tcg2.h
> +++ b/include/efi_tcg2.h
> @@ -126,7 +126,7 @@ struct efi_tcg2_boot_service_capability {
>   };
>
>   /* up to and including the vendor ID (manufacturer_id) field */
> -#define BOOT_SERVICE_CAPABILITY_MIN \
> +#define BOOT_SERVICE_CAPABILITY_1_0 \
>          offsetof(struct efi_tcg2_boot_service_capability,
> number_of_pcr_banks)
>
>   #define TCG_EFI_SPEC_ID_EVENT_SIGNATURE_03 "Spec ID Event03"
> diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
> index a83ae7a46c..d37b896b7e 100644
> --- a/lib/efi_loader/efi_tcg2.c
> +++ b/lib/efi_loader/efi_tcg2.c
> @@ -742,15 +742,15 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol
> *this,
>                  goto out;
>          }
>
> -       if (capability->size < BOOT_SERVICE_CAPABILITY_MIN) {
> -               capability->size = BOOT_SERVICE_CAPABILITY_MIN;
> +       if (capability->size < BOOT_SERVICE_CAPABILITY_1_0) {
> +               capability->size = sizeof(*capability);
>                  efi_ret = EFI_BUFFER_TOO_SMALL;
>                  goto out;
>          }
>
>          if (capability->size < sizeof(*capability)) {
>                  capability->size = sizeof(*capability);
> -               efi_ret = EFI_BUFFER_TOO_SMALL;
> +               efi_ret = EFI_UNSUPPORTED;
>                  goto out;
>          }
>
> That is:
>
> -if the passed in size is less than the 1.0 struct size
>   then return sizeof() the full struct.  This allows
>   client to query and determine the supported struct
>   size
>
> -to detect and reject 1.0 clients, if the passed in
>   size is >= 1.0 struct size AND less than sizeof(*capability)
>   then return EFI_UNSUPPORTED
>
> Does that make sense?

It does but that violates the spec once again.
The spec says that the firmware should return
BOOT_SERVICE_CAPABILITY_MIN if the size is less than the size of the
EFI_TCG2_BOOT_SERVICE_CAPABILITY.
The current logic is similar, the only thing that differs on your
patch is the size of the struct we return.

Thanks
/Ilias

>
> Thanks,
> Stuart