From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web09.52887.1643740390678192948 for ; Tue, 01 Feb 2022 10:33:11 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: stuart.yoder@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id E34C311FB; Tue, 1 Feb 2022 10:33:08 -0800 (PST) Received: from [192.168.0.129] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 84D393F40C; Tue, 1 Feb 2022 10:33:08 -0800 (PST) Message-ID: Date: Tue, 1 Feb 2022 12:32:55 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0) Gecko/20100101 Thunderbird/91.5.0 Subject: Re: [edk2-devel] [PATCH 2/6] uefi-sct/SctPkg: TCG2 Protocol: add GetCapability Test To: devel@edk2.groups.io, joseph.hemann@arm.com Cc: G Edhaya Chandran , Barton Gao , Carolyn Gjertsen , Samer El-Haj-Mahmoud , Eric Jin , Arvin Chen , Supreeth Venkatesh , Heinrich Schuchardt References: <20220109205827.3608758-1-Joseph.hemann@arm.com> <20220109205827.3608758-3-Joseph.hemann@arm.com> From: "Stuart Yoder" In-Reply-To: <20220109205827.3608758-3-Joseph.hemann@arm.com> Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit 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