Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy

["Linus Liu" <linus.liu@intel.com>]

Hi Jiewen
I add this patch into MTLS platform and collect the log.
The below is before adding patch and after adding patch. There is no warring message.


Before

InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B 67E4C490
InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385 68180030
!!! DEPRECATED INTERFACE !!! VariableLockRequestToLock() will go away soon!
!!! DEPRECATED INTERFACE !!! Please move to use Variable Policy!
!!! DEPRECATED INTERFACE !!! Variable: 737CDED7-448B-4801-B57D-B19483EC606F HddPassword
HddPasswordDxeInit(): Lock HddPassword variable (Success)


After 

InstallProtocolInterface: 09576E91-6D3F-11D2-8E39-00A0C969723B 67EA1370
InstallProtocolInterface: 330D4706-F2A0-4E4F-A369-B66FA8D54385 68153DB0
HddPasswordDxeInit(): Lock HddPassword variable (Success)


Thanks



-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com> 
Sent: Wednesday, May 3, 2023 12:11 AM
To: Liu, Linus <linus.liu@intel.com>; devel@edk2.groups.io
Cc: FST-FIR-PRC <fst-fir-prc@intel.com>; FST FIR Server <fst.fir.server@intel.com>; Chu, Maggie <maggie.chu@intel.com>
Subject: RE: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy

Thanks. The patch loos good to me.

Would you please share with us, how you validate the patch?



> -----Original Message-----
> From: Liu, Linus <linus.liu@intel.com>
> Sent: Tuesday, April 11, 2023 5:55 PM
> To: devel@edk2.groups.io
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; FST-FIR-PRC <fst-fir- 
> prc@intel.com>; FST FIR Server <fst.fir.server@intel.com>; Chu, Maggie 
> <maggie.chu@intel.com>
> Subject: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to 
> use Variable Policy
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408
> 
> Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: FST-FIR-PRC <fst-fir-prc@intel.com>
> Cc: FST FIR Server C <fst.fir.server@intel.com>
> Cc: Maggie Chu <maggie.chu@intel.com>
> Signed-off-by: Linus Liu <linus.liu@intel.com>
> ---
>  SecurityPkg/HddPassword/HddPasswordDxe.c   | 16 +++++++++++-----
>  SecurityPkg/HddPassword/HddPasswordDxe.h   |  1 -
>  SecurityPkg/HddPassword/HddPasswordDxe.inf |  3 ++-
>  SecurityPkg/SecurityPkg.dsc                |  1 +
>  4 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c
> b/SecurityPkg/HddPassword/HddPasswordDxe.c
> index a1a63b67a4..c20fdbe83f 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.c
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.c
> @@ -9,6 +9,7 @@
>  **/
> 
> 
> 
>  #include "HddPasswordDxe.h"
> 
> +#include <Library/VariablePolicyHelperLib.h>
> 
> 
> 
>  EFI_GUID    mHddPasswordVendorGuid          =
> HDD_PASSWORD_CONFIG_GUID;
> 
>  CHAR16      mHddPasswordVendorStorageName[] =
> L"HDD_PASSWORD_CONFIG";
> 
> @@ -2822,7 +2823,7 @@ HddPasswordDxeInit (
>    HDD_PASSWORD_DXE_PRIVATE_DATA  *Private;
> 
>    VOID                           *Registration;
> 
>    EFI_EVENT                      EndOfDxeEvent;
> 
> -  EDKII_VARIABLE_LOCK_PROTOCOL   *VariableLock;
> 
> +  EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy;
> 
> 
> 
>    Private = NULL;
> 
> 
> 
> @@ -2858,12 +2859,17 @@ HddPasswordDxeInit (
>    //
> 
>    // Make HDD_PASSWORD_VARIABLE_NAME variable read-only.
> 
>    //
> 
> -  Status = gBS->LocateProtocol (&gEdkiiVariableLockProtocolGuid, 
> NULL, (VOID **)&VariableLock);
> 
> +  Status = gBS->LocateProtocol (&gEdkiiVariablePolicyProtocolGuid, 
> + NULL,
> (VOID **)&VariablePolicy);
> 
>    if (!EFI_ERROR (Status)) {
> 
> -    Status = VariableLock->RequestToLock (
> 
> -                             VariableLock,
> 
> +    Status = RegisterBasicVariablePolicy (
> 
> +                             VariablePolicy,
> 
> +                             &mHddPasswordVendorGuid,
> 
>                               HDD_PASSWORD_VARIABLE_NAME,
> 
> -                             &mHddPasswordVendorGuid
> 
> +                             VARIABLE_POLICY_NO_MIN_SIZE,
> 
> +                             VARIABLE_POLICY_NO_MAX_SIZE,
> 
> +                             VARIABLE_POLICY_NO_MUST_ATTR,
> 
> +                             VARIABLE_POLICY_NO_CANT_ATTR,
> 
> +                             VARIABLE_POLICY_TYPE_LOCK_NOW
> 
>                               );
> 
>      DEBUG ((DEBUG_INFO, "%a(): Lock %s variable (%r)\n", 
> __FUNCTION__, HDD_PASSWORD_VARIABLE_NAME, Status));
> 
>      ASSERT_EFI_ERROR (Status);
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.h
> b/SecurityPkg/HddPassword/HddPasswordDxe.h
> index 231533e737..049a208794 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.h
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.h
> @@ -17,7 +17,6 @@
>  #include <Protocol/AtaPassThru.h>
> 
>  #include <Protocol/PciIo.h>
> 
>  #include <Protocol/HiiConfigAccess.h>
> 
> -#include <Protocol/VariableLock.h>
> 
> 
> 
>  #include <Guid/MdeModuleHii.h>
> 
>  #include <Guid/EventGroup.h>
> 
> diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> index 06e8755ffc..2c0ebbcc78 100644
> --- a/SecurityPkg/HddPassword/HddPasswordDxe.inf
> +++ b/SecurityPkg/HddPassword/HddPasswordDxe.inf
> @@ -50,6 +50,7 @@
>    PrintLib
> 
>    UefiLib
> 
>    LockBoxLib
> 
> +  VariablePolicyHelperLib
> 
>    S3BootScriptLib
> 
>    PciLib
> 
>    BaseCryptLib
> 
> @@ -63,7 +64,7 @@
>    gEfiHiiConfigAccessProtocolGuid               ## PRODUCES
> 
>    gEfiAtaPassThruProtocolGuid                   ## CONSUMES
> 
>    gEfiPciIoProtocolGuid                         ## CONSUMES
> 
> -  gEdkiiVariableLockProtocolGuid                ## CONSUMES
> 
> +  gEdkiiVariablePolicyProtocolGuid              ## CONSUMES
> 
> 
> 
>  [Pcd]
> 
>    gEfiSecurityPkgTokenSpaceGuid.PcdSkipHddPasswordPrompt  ## CONSUMES
> 
> diff --git a/SecurityPkg/SecurityPkg.dsc b/SecurityPkg/SecurityPkg.dsc 
> index 3bad5375c0..3c62205162 100644
> --- a/SecurityPkg/SecurityPkg.dsc
> +++ b/SecurityPkg/SecurityPkg.dsc
> @@ -74,6 +74,7 @@
> 
> PlatformPKProtectionLib|SecurityPkg/Library/PlatformPKProtectionLibVar
> PlatformPKProtectionLib|Po
> licy/PlatformPKProtectionLibVarPolicy.inf
> 
> 
> SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableP
> SecureBootVariableProvisionLib|ro
> visionLib/SecureBootVariableProvisionLib.inf
> 
>    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> 
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/V
> VariablePolicyHelperLib|ar
> iablePolicyHelperLib.inf
> 
> 
> 
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> 
>    #
> 
> --
> 2.33.1.windows.1