Hi Bret
Since it took much long time to get ECC feedback than I expected, I would give feedback on code while we are waiting.
1) Please confirm how you test the code, such as Microsoft platform ?
2) Please remove “+} // Tpm2NvUndefineSpaceSpecial()” at the end of the function. We do not use that style in other code.
3) Please copy the definition from TPM spec “This command allows removal of a platform-created NV Index that has TPMA_NV_POLICY_DELETE SET” to the function header description. The current one “This command removes an index from the TPM.” is for TPM2_NV_UndefineSpace instead of TPM2_NV_UndefineSpaceSpecial.
Since above comment does not impact any function, I would like to give RB.
With about change, reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
Thank you
Yao Jiewen
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:
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 <bret.barkelew@microsoft.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Qi Zhang <qi1.zhang@intel.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
---
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]
-=-=-=-=-=-=