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

[Stuart Yoder]

Unfortunately, the TCG spec is very confusing in section 6.4.4 #2 and
#3.  They attempted to clarify in an errata:
https://trustedcomputinggroup.org/wp-content/uploads/EFI-Protocol-Specification-Errata-v.5.pdf

...but it is still confusing.

Ilias and I had discussed the ambiguities, and back in March 2022 I
requested clarification from the TCG workgroup.  In cases of
ambiguity TCG frequently will defer to how EDK2 has implemented
a point in the spec.

Here are my notes following the call with TCG about the intent
of #2 and #3, which was based on their review of the EDK2
implementation:

a. If a client passes in a Size that is the full size including all
    fields including ActivePcrBanks, the return code is SUCCESS and
    all fields are populated. [This is a 1.1 client scenario]

b. If a client passes in a Size that includes all fields up to
    and including the vendor ID, the return code is SUCCESS and all
    fields up to including the vendor ID are populated. [This is a
    1.0 client scenario, so a populated 1.0 struct is returned]

c. If a client passes in a Size that is less than the size up to
    and including the vendor ID, the return code is BUFFER_TOO_SMALL
    and the Size field is populated with the full size of the struct
    supported by the firmware. [This allows a client to determine
    whether it is talking to 1.0 or 1.1 firmware]

What Ilias wants to avoid is supporting #b above in u-boot, and
I agree that having u-boot support clients based on struct 1.0
seems unnecessary.

>> 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?
> 
> No, this will not work. A 1.0 client may call with size = 0 and next
> will call with whatever size was returned accompanied by
> EFI_BUFFER_TOO_SMALL.

The spec defines the Size field as "Allocated size of the
structure.", so I don't see how a value of 0 makes sense.

If a client wants to just get the Size field populated, pass in
a buffer of Size=1 and firmware will return just the Size field
populated with the correct struct size.

> It is the client's obligation to check the values of fields
> StructureVersion and ProtocolVersion.

Yes, but what if the client fails to do that? What if a client
passes in a struct with a size corresponding to a 1.0 struct?
What should firmware do?

According to point #b above, firmware should support the 1.0
client and populate only the 1.0 fields.  But, what we are
proposing is not supporting that in u-boot.  Instead return
EFI_UNSUPPORTED.  I can discuss this with the TCG WG.

Thanks,
Stuart