Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1] SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib

["Bret Barkelew" <bret@corthon.com>]

[-- Attachment #1: Type: text/plain, Size: 16787 bytes --]

Good question! I had a kid in September and then everything fell off my
radar. I think I can find this branch and knock out a second patch, though.

On Fri, Oct 8, 2021 at 7:31 PM Yao, Jiewen <jiewen.yao@intel.com> wrote:

> Hey
>
> Is that any update for this patch?
>
>
>
> I did not see the v2 patch. Just want to ensure I did not miss that by
> mistake.
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> *From:* Bret Barkelew <Bret.Barkelew@microsoft.com>
> *Sent:* Friday, August 13, 2021 11:22 AM
> *To:* Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io;
> gaoliming@byosoft.com.cn; bret@corthon.com; Kinney, Michael D <
> michael.d.kinney@intel.com>
> *Cc:* Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <
> qi1.zhang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> *Subject:* Re: [EXTERNAL] [edk2-devel] [PATCH v1 1/1]
> SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib
>
>
>
> Thanks, Jiewen! I’ll make those changes!
>
>
>
> - Bret
> ------------------------------
>
> *From:* Yao, Jiewen <jiewen.yao@intel.com>
> *Sent:* Thursday, August 12, 2021 7:47:04 PM
> *To:* devel@edk2.groups.io <devel@edk2.groups.io>; Yao, Jiewen <
> jiewen.yao@intel.com>; gaoliming@byosoft.com.cn <gaoliming@byosoft.com.cn>;
> Bret Barkelew <Bret.Barkelew@microsoft.com>; bret@corthon.com <
> bret@corthon.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> *Cc:* Wang, Jian J <jian.j.wang@intel.com>; Zhang, Qi1 <
> qi1.zhang@intel.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
> *Subject:* RE: [EXTERNAL] [edk2-devel] [PATCH v1 1/1]
> SecurityPkg/Library: Add Tpm2NvUndefineSpaceSpecial to Tpm2CommandLib
>
>
>
> 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
> <bret.barkelew=microsoft.com@groups.io>
> *Sent: *Friday, July 30, 2021 11:08 AM
> *To: *devel@edk2.groups.io; bret@corthon.com
> *Cc: *Yao, Jiewen <jiewen.yao@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Qi Zhang <qi1.zhang@intel.com>; Rahul Kumar
> <rahul1.kumar@intel.com>
> *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)
> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftianocore%2Fedk2%2Fpull%2F1848&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C887b6ef599664679e21808d95e04a428%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637644196321232497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=1TMpHKJ9Mwh6VW4fBAjs0Cf2hIqcmbtDfNqsJY47z5U%3D&reserved=0>
>
>
>
> 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 <bret=corthon.com@groups.io>
> *Sent: *Friday, July 30, 2021 10:55 AM
> *To: *devel@edk2.groups.io
> *Cc: *Yao, Jiewen <jiewen.yao@intel.com>; Jian J Wang
> <jian.j.wang@intel.com>; Qi Zhang <qi1.zhang@intel.com>; Rahul Kumar
> <rahul1.kumar@intel.com>
> *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&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397602953%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=s96M3RvxMOY831Vfr1nt%2Fz1h3cyb6jU9eFzvjKO7Dtc%3D&amp;reserved=0
> <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%7C887b6ef599664679e21808d95e04a428%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637644196321232497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=q%2FGqsG776H1TD9bqGRw8ihZNDnJscXK0dwzEVGCNYf0%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&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=CWxLwgp73z2XQEa%2FN77gsCwRF73xha0RZCKwcFTlrRE%3D&amp;reserved=0
> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F78450&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C887b6ef599664679e21808d95e04a428%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637644196321232497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=9RfO5tO2iBj%2BR7eTmtcOkdgCyIXFOCVKtqxRt3sWNUA%3D&reserved=0>
> Mute This Topic:
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555713%2F1822150&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=IWQ6E4yP0ECt3oYLYQa%2BnddGfcQEDMgfASlcxRuda%2BQ%3D&amp;reserved=0
> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F84555713%2F1822150&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C887b6ef599664679e21808d95e04a428%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637644196321242452%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=C1krPlgEdkk%2FZ9PLVv6e3AwJCFAI%2BYl1uYU0kGXmkyY%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&amp;data=04%7C01%7CBret.Barkelew%40microsoft.com%7Cb7ae3c62047c48fc85d908d953833ca0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637632645397612922%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=qor4Y5FZEH8ch0AEmWDbe97FIQk4V1qx7IURcTHzjAU%3D&amp;reserved=0
> <https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C887b6ef599664679e21808d95e04a428%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637644196321242452%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=%2FKgT0i6BOB11aRu2kw8yFoQ8y1jcg1eOgzg8CSEGXUA%3D&reserved=0>
> [brbarkel@microsoft.com]
> -=-=-=-=-=-=
>
>
>
>
>
>
>
> 
>

[-- Attachment #2: Type: text/html, Size: 28356 bytes --]