Jiewen: Yes. If the definition is to align the industry standard definition, I am OK to add them into ECC exception. Thanks Liming 发件人: devel@edk2.groups.io 代表 Yao, Jiewen 发送时间: 2021年8月13日 10:30 收件人: devel@edk2.groups.io; gaoliming@byosoft.com.cn; bret.barkelew@microsoft.com; bret@corthon.com; Kinney, Michael D 抄送: Wang, Jian J ; Zhang, Qi1 ; Kumar, Rahul1 主题: Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Hi I have seen such exception in the industry standard file, such as https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Tpm20.h #define TPM_CC_NV_UndefineSpaceSpecial (TPM_CC)(0x0000011F) #define TPM_CC_EvictControl (TPM_CC)(0x00000120) #define TPM_CC_HierarchyControl (TPM_CC)(0x00000121) #define TPM_CC_NV_UndefineSpace (TPM_CC)(0x00000122) #define TPM_CC_ChangeEPS (TPM_CC)(0x00000124) #define TPM_CC_ChangePPS (TPM_CC)(0x00000125) #define TPM_CC_Clear (TPM_CC)(0x00000126) #define TPM_CC_ClearControl (TPM_CC)(0x00000127) #define TPM_CC_ClockSet (TPM_CC)(0x00000128) #define TPM_CC_HierarchyChangeAuth (TPM_CC)(0x00000129) #define TPM_CC_NV_DefineSpace (TPM_CC)(0x0000012A) #define TPM_CC_PCR_Allocate (TPM_CC)(0x0000012B) #define TPM_CC_PCR_SetAuthPolicy (TPM_CC)(0x0000012C) #define TPM_CC_PP_Commands (TPM_CC)(0x0000012D) #define TPM_CC_SetPrimaryPolicy (TPM_CC)(0x0000012E) #define TPM_CC_FieldUpgradeStart (TPM_CC)(0x0000012F) #define TPM_CC_ClockRateAdjust (TPM_CC)(0x00000130) https://github.com/tianocore/edk2/blob/master/MdePkg/Include/IndustryStandard/Tpm12.h #define TPM_ORD_ActivateIdentity ((TPM_COMMAND_CODE) 0x0000007A) #define TPM_ORD_AuthorizeMigrationKey ((TPM_COMMAND_CODE) 0x0000002B) #define TPM_ORD_CertifyKey ((TPM_COMMAND_CODE) 0x00000032) #define TPM_ORD_CertifyKey2 ((TPM_COMMAND_CODE) 0x00000033) #define TPM_ORD_CertifySelfTest ((TPM_COMMAND_CODE) 0x00000052) #define TPM_ORD_ChangeAuth ((TPM_COMMAND_CODE) 0x0000000C) #define TPM_ORD_ChangeAuthAsymFinish ((TPM_COMMAND_CODE) 0x0000000F) #define TPM_ORD_ChangeAuthAsymStart ((TPM_COMMAND_CODE) 0x0000000E) #define TPM_ORD_ChangeAuthOwner ((TPM_COMMAND_CODE) 0x00000010) I agree with Liming that if this is something completely defined by EDKII, we should align with EDKII. But if this is something copied from other standard, I prefer we keep them as is and add those to exception list. That makes easy for the domain specific expert to review the code. Can we submit patch to add those to ECC exception list? Thank you Yao Jiewen From: devel@edk2.groups.io > On Behalf Of gaoliming Sent: Friday, August 13, 2021 9:38 AM To: devel@edk2.groups.io ; bret.barkelew@microsoft.com ; Yao, Jiewen >; bret@corthon.com ; Kinney, Michael D > Cc: Wang, Jian J >; Zhang, Qi1 >; Kumar, Rahul1 > Subject: 回复: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Bret: I suggest to define new macro that follows EDKII style. This MACRO is only used in CommandLib. Its impact should be small. Thanks Liming 发件人: devel@edk2.groups.io > 代表 Bret Barkelew via groups.io 发送时间: 2021年8月13日 8:24 收件人: Yao, Jiewen >; devel@edk2.groups.io ; bret@corthon.com ; Kinney, Michael D > 抄送: Wang, Jian J >; Zhang, Qi1 >; Kumar, Rahul1 > 主题: Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib + @Kinney, Michael D Mike, any thoughts on when “sticking with the file convention” breaks ECC? Should I just ignore the rest of the file and pass ECC at all costs? - Bret From: Yao, Jiewen Sent: Thursday, August 12, 2021 5:06 PM To: Bret Barkelew ; devel@edk2.groups.io ; bret@corthon.com Cc: Wang, Jian J ; Zhang, Qi1 ; Kumar, Rahul1 Subject: RE: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib I don’t know the answer about ECC or PatchChecker. I just know we need pass CI to merge the patch. Thank you Yao Jiewen From: Bret Barkelew > Sent: Friday, August 13, 2021 1:52 AM To: devel@edk2.groups.io ; bret@corthon.com Cc: Yao, Jiewen >; Wang, Jian J >; Zhang, Qi1 >; Kumar, Rahul1 > Subject: RE: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Thoughts? - Bret From: devel@edk2.groups.io > on behalf of Bret Barkelew via groups.io > Sent: Wednesday, August 11, 2021 12:37:52 PM To: devel@edk2.groups.io >; bret@corthon.com > Cc: Yao, Jiewen >; Jian J Wang >; Qi Zhang >; Rahul Kumar > Subject: Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Thoughts? - Bret From: devel@edk2.groups.io > on behalf of Bret Barkelew via groups.io > Sent: Wednesday, August 4, 2021 9:32:32 AM To: devel@edk2.groups.io >; bret@corthon.com > Cc: Yao, Jiewen >; Jian J Wang >; Qi Zhang >; Rahul Kumar > Subject: Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Poking this one. 1. It’s a easy review with small, obvious code change. 2. I need some answers on “when is it okay to violate ECC/PatchCheck, if the new code matches the style of the existing code. Should I endeavor to pass the PatchCheck and ECCCheck with this patch only, and leave it in conflict with the rest of the file? Thanks! - Bret From: Bret Barkelew via groups.io Sent: Friday, July 30, 2021 11:08 AM To: devel@edk2.groups.io ; bret@corthon.com Cc: Yao, Jiewen ; Jian J Wang ; Qi Zhang ; Rahul Kumar Subject: Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Note, even though this keeps with the style of the rest of the file, it breaks ECC: SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib by corthon · Pull Request #1848 · tianocore/edk2 (github.com) PROGRESS - --Running SecurityPkg: EccCheck Test NO-TARGET -- ERROR - ERROR - ERROR - EFI coding style error ERROR - *Error code: 8001 ERROR - *Only capital letters are allowed to be used for #define declarations ERROR - *file: //home/vsts/work/1/s/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c ERROR - *Line number: 27 ERROR - *The #define name [RC_NV_UndefineSpaceSpecial_nvIndex] does no Thoughts? - Bret From: Bret Barkelew via groups.io Sent: Friday, July 30, 2021 10:55 AM To: devel@edk2.groups.io Cc: Yao, Jiewen ; Jian J Wang ; Qi Zhang ; Rahul Kumar Subject: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib Used to provision and maintain certain HW-defined NV spaces. REF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2994 &data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397602953%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=s96M3RvxMOY831Vfr1nt%2Fz1h3cyb6jU9eFzvjKO7Dtc%3D&reserved=0 Signed-off-by: Bret Barkelew > Cc: Jiewen Yao > Cc: Jian J Wang > Cc: Qi Zhang > Cc: Rahul Kumar > --- SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c | 122 ++++++++++++++++++++ SecurityPkg/Include/Library/Tpm2CommandLib.h | 22 ++++ 2 files changed, 144 insertions(+) diff --git a/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c b/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c index 87572de20164..7931fade9190 100644 --- a/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c +++ b/SecurityPkg/Library/Tpm2CommandLib/Tpm2NVStorage.c @@ -24,6 +24,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent #define RC_NV_UndefineSpace_authHandle (TPM_RC_H + TPM_RC_1) #define RC_NV_UndefineSpace_nvIndex (TPM_RC_H + TPM_RC_2) +#define RC_NV_UndefineSpaceSpecial_nvIndex (TPM_RC_H + TPM_RC_1) + #define RC_NV_Read_authHandle (TPM_RC_H + TPM_RC_1) #define RC_NV_Read_nvIndex (TPM_RC_H + TPM_RC_2) #define RC_NV_Read_size (TPM_RC_P + TPM_RC_1) @@ -74,6 +76,20 @@ typedef struct { TPMS_AUTH_RESPONSE AuthSession; } TPM2_NV_UNDEFINESPACE_RESPONSE; +typedef struct { + TPM2_COMMAND_HEADER Header; + TPMI_RH_NV_INDEX NvIndex; + TPMI_RH_PLATFORM Platform; + UINT32 AuthSessionSize; + TPMS_AUTH_COMMAND AuthSession; +} TPM2_NV_UNDEFINESPACESPECIAL_COMMAND; + +typedef struct { + TPM2_RESPONSE_HEADER Header; + UINT32 AuthSessionSize; + TPMS_AUTH_RESPONSE AuthSession; +} TPM2_NV_UNDEFINESPACESPECIAL_RESPONSE; + typedef struct { TPM2_COMMAND_HEADER Header; TPMI_RH_NV_AUTH AuthHandle; @@ -506,6 +522,112 @@ Done: return Status; } +/** + This command removes an index from the TPM. + + @param[in] NvIndex The NV Index. + @param[in] IndexAuthSession Auth session context for the Index auth/policy + @param[in] PlatAuthSession Auth session context for the Platform auth/policy + + @retval EFI_SUCCESS Operation completed successfully. + @retval EFI_NOT_FOUND The command was returned successfully, but NvIndex is not found. + @retval EFI_UNSUPPORTED Selected NvIndex does not support deletion through this call. + @retval EFI_SECURITY_VIOLATION Deletion is not authorized by current policy session. + @retval EFI_INVALID_PARAMETER The command was unsuccessful. + @retval EFI_DEVICE_ERROR The command was unsuccessful. +**/ +EFI_STATUS +EFIAPI +Tpm2NvUndefineSpaceSpecial ( + IN TPMI_RH_NV_INDEX NvIndex, + IN TPMS_AUTH_COMMAND *IndexAuthSession OPTIONAL, + IN TPMS_AUTH_COMMAND *PlatAuthSession OPTIONAL + ) +{ + EFI_STATUS Status; + TPM2_NV_UNDEFINESPACESPECIAL_COMMAND SendBuffer; + TPM2_NV_UNDEFINESPACESPECIAL_RESPONSE RecvBuffer; + UINT32 SendBufferSize; + UINT32 RecvBufferSize; + UINT8 *Buffer; + UINT32 IndexAuthSize, PlatAuthSize; + TPM_RC ResponseCode; + + // + // Construct command + // + SendBuffer.Header.tag = SwapBytes16(TPM_ST_SESSIONS); + SendBuffer.Header.commandCode = SwapBytes32(TPM_CC_NV_UndefineSpaceSpecial); + + SendBuffer.NvIndex = SwapBytes32 (NvIndex); + SendBuffer.Platform = SwapBytes32 (TPM_RH_PLATFORM); + + // + // Marshall the Auth Sessions for the two handles. + Buffer = (UINT8 *)&SendBuffer.AuthSession; + // IndexAuthSession + IndexAuthSize = CopyAuthSessionCommand (IndexAuthSession, Buffer); + Buffer += IndexAuthSize; + // PlatAuthSession + PlatAuthSize = CopyAuthSessionCommand (PlatAuthSession, Buffer); + Buffer += PlatAuthSize; + // AuthSessionSize + SendBuffer.AuthSessionSize = SwapBytes32(IndexAuthSize + PlatAuthSize); + + // Update total command size. + SendBufferSize = (UINT32)(Buffer - (UINT8 *)&SendBuffer); + SendBuffer.Header.paramSize = SwapBytes32 (SendBufferSize); + + // + // send Tpm command + // + RecvBufferSize = sizeof (RecvBuffer); + Status = Tpm2SubmitCommand (SendBufferSize, (UINT8 *)&SendBuffer, &RecvBufferSize, (UINT8 *)&RecvBuffer); + if (EFI_ERROR (Status)) { + goto Done; + } + + if (RecvBufferSize < sizeof (TPM2_RESPONSE_HEADER)) { + DEBUG ((EFI_D_ERROR, "Tpm2NvUndefineSpaceSpecial - RecvBufferSize Error - %x\n", RecvBufferSize)); + Status = EFI_DEVICE_ERROR; + goto Done; + } + + ResponseCode = SwapBytes32(RecvBuffer.Header.responseCode); + if (ResponseCode != TPM_RC_SUCCESS) { + DEBUG ((EFI_D_ERROR, "Tpm2NvUndefineSpaceSpecial - responseCode - %x\n", SwapBytes32(RecvBuffer.Header.responseCode))); + } + switch (ResponseCode) { + case TPM_RC_SUCCESS: + // return data + break; + case TPM_RC_ATTRIBUTES: + case TPM_RC_ATTRIBUTES + RC_NV_UndefineSpaceSpecial_nvIndex: + Status = EFI_UNSUPPORTED; + break; + case TPM_RC_NV_AUTHORIZATION: + Status = EFI_SECURITY_VIOLATION; + break; + case TPM_RC_HANDLE + RC_NV_UndefineSpaceSpecial_nvIndex: // TPM_RC_NV_DEFINED: + Status = EFI_NOT_FOUND; + break; + case TPM_RC_VALUE + RC_NV_UndefineSpace_nvIndex: + Status = EFI_INVALID_PARAMETER; + break; + default: + Status = EFI_DEVICE_ERROR; + break; + } + +Done: + // + // Clear AuthSession Content + // + ZeroMem (&SendBuffer, sizeof(SendBuffer)); + ZeroMem (&RecvBuffer, sizeof(RecvBuffer)); + return Status; +} // Tpm2NvUndefineSpaceSpecial() + /** This command reads a value from an area in NV memory previously defined by TPM2_NV_DefineSpace(). diff --git a/SecurityPkg/Include/Library/Tpm2CommandLib.h b/SecurityPkg/Include/Library/Tpm2CommandLib.h index ee8eb622951c..8d7b4998d98d 100644 --- a/SecurityPkg/Include/Library/Tpm2CommandLib.h +++ b/SecurityPkg/Include/Library/Tpm2CommandLib.h @@ -364,6 +364,28 @@ Tpm2NvUndefineSpace ( IN TPMS_AUTH_COMMAND *AuthSession OPTIONAL ); +/** + This command removes an index from the TPM. + + @param[in] NvIndex The NV Index. + @param[in] IndexAuthSession Auth session context for the Index auth/policy + @param[in] PlatAuthSession Auth session context for the Platform auth/policy + + @retval EFI_SUCCESS Operation completed successfully. + @retval EFI_NOT_FOUND The command was returned successfully, but NvIndex is not found. + @retval EFI_UNSUPPORTED Selected NvIndex does not support deletion through this call. + @retval EFI_SECURITY_VIOLATION Deletion is not authorized by current policy session. + @retval EFI_INVALID_PARAMETER The command was unsuccessful. + @retval EFI_DEVICE_ERROR The command was unsuccessful. +**/ +EFI_STATUS +EFIAPI +Tpm2NvUndefineSpaceSpecial ( + IN TPMI_RH_NV_INDEX NvIndex, + IN TPMS_AUTH_COMMAND *IndexAuthSession OPTIONAL, + IN TPMS_AUTH_COMMAND *PlatAuthSession OPTIONAL + ); + /** This command reads a value from an area in NV memory previously defined by TPM2_NV_DefineSpace(). -- 2.31.1.windows.1 -=-=-=-=-=-= Groups.io Links: You receive all messages sent to this group. View/Reply Online (#78450): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78450 &data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=CWxLwgp73z2XQEa%2FN77gsCwRF73xha0RZCKwcFTlrRE%3D&reserved=0 Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555713%2F1822150 &data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=IWQ6E4yP0ECt3oYLYQa%2BnddGfcQEDMgfASlcxRuda%2BQ%3D&reserved=0 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub &data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=qor4Y5FZEH8ch0AEmWDbe97FIQk4V1qx7IURcTHzjAU%3D&reserved=0 [brbarkel@microsoft.com] -=-=-=-=-=-=