Re: [edk2-devel] [PATCH 2/6] uefi-sct/SctPkg: TCG2 Protocol: add GetCapability Test

["Stuart Yoder" <stuart.yoder@arm.com>]

See inline comments...

> +EFI_STATUS
> +BBTestGetCapabilityConformanceTestCheckpoint2 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_TCG2_PROTOCOL                     *TCG2
> +  )
> +{
> +  EFI_TEST_ASSERTION                    AssertionType;
> +  EFI_STATUS                            Status;
> +  char StructureVersionMajor;
> +  char StructureVersionMinor;
> +  char ProtocolVersionMajor;
> +  char ProtocolVersionMinor;
> +
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY      BootServiceCap;
> +  BootServiceCap.Size = sizeof(UINT8) + (sizeof(EFI_TCG2_VERSION) * 2);
> +
> +  Status = TCG2->GetCapability (
> +                           TCG2,
> +                           &BootServiceCap);
> +
> +  AssertionType = EFI_TEST_ASSERTION_PASSED;
> +
> +  // If the input ProtocolCapability.Size < sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY) the function should return EFI_BUFFER_TOO_SMALL
> +  if (Status != EFI_BUFFER_TOO_SMALL) {
> +     StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: Did not return Status == EFI_BUFFER_TOO_SMALL with input ProtocolCapability.Size < sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY)"
> +                     );
> +
> +     AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  StructureVersionMajor = BootServiceCap.StructureVersion.Major;
> +  StructureVersionMinor = BootServiceCap.StructureVersion.Minor;
> +
> +  // If the input ProtocolCapability.Size < sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY) the function will initialize the fields included in ProtocolCapability.Size.
> +  if ((StructureVersionMajor != 1) | (StructureVersionMinor != 1)) {
> +     StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: Unexpected struct version numbers returned"
> +                     );
> +
> +     AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  ProtocolVersionMajor = BootServiceCap.ProtocolVersion.Major;
> +  ProtocolVersionMinor = BootServiceCap.ProtocolVersion.Minor;
> +
> +  if ((ProtocolVersionMajor != 1) | (ProtocolVersionMinor != 1)) {
> +     StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: Unexpected protocol version numbers returned."
> +                     );
> +
> +     AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  StandardLib->RecordAssertion (
> +                 StandardLib,
> +                 AssertionType,
> +                 gTcg2ConformanceTestAssertionGuid002,
> +                 L"TCG2_PROTOCOL.GetCapability() - GetCapability() shall populate the included structure elements and return with a Status of EFI_BUFFER_TOO_SMALL when structure size is set to less than the size of EFI_TCG_BOOT_SERVICE_CAPABILITY.",
> +                 L"%a:%d: Status - %r",
> +                 __FILE__,
> +                 (UINTN)__LINE__,
> +                 Status
> +                 );

In the SCT spec draft there is a test:

   f. Verify returned Size equal to size of the
   EFI_TCG2_BOOT_SERVICE_CAPABILITY up to and including the vendor ID
   field.

...but I don't see that test covered in the code.

> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +BBTestGetCapabilityConformanceTestCheckpoint3 (
> +  IN EFI_STANDARD_TEST_LIBRARY_PROTOCOL    *StandardLib,
> +  IN EFI_TCG2_PROTOCOL                     *TCG2
> +  )
> +{
> +  EFI_TEST_ASSERTION                    AssertionType;
> +  EFI_STATUS                            Status;
> +  char StructureVersionMajor;
> +  char StructureVersionMinor;
> +  char ProtocolVersionMajor;
> +  char ProtocolVersionMinor;
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY      BootServiceCap;
> +
> +  BootServiceCap.Size = sizeof(EFI_TCG2_BOOT_SERVICE_CAPABILITY);

BootServiceCap.Size is not used in this test, so should be able to 
delete the above line.

> +  Status = TCG2->GetCapability (
> +                           TCG2,
> +                           &BootServiceCap);
> +
> +  AssertionType = EFI_TEST_ASSERTION_PASSED;
> +
> +  if (Status != EFI_SUCCESS) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: GetCapabilty should return EFI_SUCCESS"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  StructureVersionMajor = BootServiceCap.StructureVersion.Major;
> +  StructureVersionMinor = BootServiceCap.StructureVersion.Minor;
> +
> +  // TCG EFI Protocol spec 6.4.4 #4
> +  if ((StructureVersionMajor != 1) | (StructureVersionMinor != 1)) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: GetCapabilty should have StructureVersion 1.1"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  ProtocolVersionMajor = BootServiceCap.ProtocolVersion.Major;
> +  ProtocolVersionMinor = BootServiceCap.ProtocolVersion.Minor;
> +
> +  // TCG EFI Protocol spec 6.4.4 #4
> +  if ((ProtocolVersionMajor != 1) | (ProtocolVersionMinor != 1)) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: protocol version must be 1.1"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  if (!(BootServiceCap.SupportedEventLogs &  EFI_TCG2_EVENT_LOG_FORMAT_TCG_2)) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: GetCapabilty must support TCG2 event log format"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  if (BootServiceCap.NumberOfPcrBanks < 1 ) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: expect at least 1 PCR bank"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  if (!(BootServiceCap.HashAlgorithmBitmap & EFI_TCG2_BOOT_HASH_ALG_SHA256)) {
> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: unexpected hash algorithms reported = %x",
> +                     BootServiceCap.HashAlgorithmBitmap
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  if (!(~BootServiceCap.ActivePcrBanks & BootServiceCap.HashAlgorithmBitmap) == 0) {

The above test doesn't look correct to me.  Need to sanity check that.

> +    StandardLib->RecordMessage (
> 
> +                     StandardLib,
> 
> +                     EFI_VERBOSE_LEVEL_DEFAULT,
> 
> +                     L"\r\nTCG2 Protocol GetCapablity Test: ActivePcrBanks is not a subset of HashAlgorithmBitmap"
> +                     );
> +
> +    AssertionType = EFI_TEST_ASSERTION_FAILED;
> +  }
> +
> +  if (!(BootServiceCap.ActivePcrBanks & EFI_TCG2_BOOT_HASH_ALG_SHA256)) {

In the SCT spec the test says to verify that ActivePcrBanks includes 
SHA256, SHA384, or SHA512.  As written the test would fail if active pcr 
banks was SHA384.

Thanks,
Stuart