From: "Joseph Hemann" <joseph.hemann@arm.com>
To: Stuart Yoder <Stuart.Yoder@arm.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: G Edhaya Chandran <Edhaya.Chandran@arm.com>,
Barton Gao <gaojie@byosoft.com.cn>,
Carolyn Gjertsen <Carolyn.Gjertsen@amd.com>,
Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>,
Eric Jin <eric.jin@intel.com>, Arvin Chen <arvinx.chen@intel.com>,
Supreeth Venkatesh <Supreeth.Venkatesh@amd.com>,
Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Subject: Re: [edk2-devel] [PATCH 2/6] uefi-sct/SctPkg: TCG2 Protocol: add GetCapability Test
Date: Mon, 7 Feb 2022 17:59:14 +0000 [thread overview]
Message-ID: <0232AA76-0DBC-45EA-8379-3A4DF2881E79@arm.com> (raw)
In-Reply-To: <b7e40226-4949-5d69-bea6-8960f02019b7@arm.com>
See inline comments..
On 2/1/22, 12:33 PM, "Stuart Yoder" <stuart.yoder@arm.com> wrote:
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.
I will add this check to the next round of edits I send out.
> + 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.
BootServiceCap.Size needs to be set so that GetCapability knows how much of the struct to fill out. Even though it is not checked it still needs to be set before calling GetCapabilty.
> + 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.
After looking at it further I agree. The check should look something like ((BootServiceCap.ActivePcrBanks & ~BootServiceCap.HashAlgorithmBitmap) != 0)
> + 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.
I have decided to change the spec to only check for SHA_256 in this case. 384 and 512 should only be implemented if 256 is implemented but we wont check for that.
Thanks,
Joseph
On 2/1/22, 12:33 PM, "Stuart Yoder" <stuart.yoder@arm.com> wrote:
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
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
next prev parent reply other threads:[~2022-02-07 17:59 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-09 20:58 [PATCH V2 0/6] Implementation of TCG2 Protocol test Joseph Hemann
2022-01-09 20:58 ` [PATCH 1/6] uefi-sct/SctPkg: TCG2 Protocol: add header with TCG2 protocol definitions Joseph Hemann
2022-01-09 20:58 ` [PATCH 2/6] uefi-sct/SctPkg: TCG2 Protocol: add GetCapability Test Joseph Hemann
2022-02-01 18:32 ` [edk2-devel] " Stuart Yoder
2022-02-07 17:59 ` Joseph Hemann [this message]
2022-01-09 20:58 ` [PATCH 3/6] uefi-sct/SctPkg: TCG2 Protocol: add GetActivePcrBanks test Joseph Hemann
2022-01-09 20:58 ` [PATCH 4/6] uefi-sct/SctPkg: TCG2 Protocol: add HashLogExtendEvent test Joseph Hemann
2022-01-09 20:58 ` [PATCH 5/6] uefi-sct/SctPkg: TCG2 Protocol: add GetEventLog test Joseph Hemann
2022-01-09 20:58 ` [PATCH 6/6] uefi-sct/SctPkg: TCG2 Protocol: add SubmitCommand test Joseph Hemann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=0232AA76-0DBC-45EA-8379-3A4DF2881E79@arm.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox