[edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy

Linus Liu posted 1 patch 1 year ago
Failed in applying to current master (apply log)
There is a newer version of this series
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(-)
[edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Linus Liu 1 year ago
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/PlatformPKProtectionLibVarPolicy/PlatformPKProtectionLibVarPolicy.inf
   SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariableProvisionLib/SecureBootVariableProvisionLib.inf
   TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
+  VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf
 
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
   #
-- 
2.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#102900): https://edk2.groups.io/g/devel/message/102900
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Yao, Jiewen 12 months ago
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/PlatformPKProtectionLibVarPo
> licy/PlatformPKProtectionLibVarPolicy.inf
> 
> 
> SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariablePro
> visionLib/SecureBootVariableProvisionLib.inf
> 
>    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> 
> +
> VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/Var
> iablePolicyHelperLib.inf
> 
> 
> 
>  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> 
>    #
> 
> --
> 2.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103861): https://edk2.groups.io/g/devel/message/103861
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Linus Liu 12 months ago
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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103868): https://edk2.groups.io/g/devel/message/103868
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Yao, Jiewen 12 months ago
That only proves that you did change the interface. But that cannot prove you change it right.

Have you done any function test? For example:
1) The HDD password feature still works?
2) The variable is really locked?


> -----Original Message-----
> From: Liu, Linus <linus.liu@intel.com>
> Sent: Wednesday, May 3, 2023 8:40 AM
> To: Yao, Jiewen <jiewen.yao@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
> 
> 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103871): https://edk2.groups.io/g/devel/message/103871
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Linus Liu 12 months ago
Hi Jieewn
Please refer the below reply.

Have you done any function test? For example:
1) The HDD password feature still works?
 Linus : yes , HDD password feature still works.

2) The variable is really locked?
 Linus : I've tried using dmpstore command to write HDDPassword in UEFI Shell. Can't override it.
 
Please refer to the below log. 
[2023-05-04 11:42:11.046] FS1:\> dmpstore -guid 737cded7-448b-4801-b57d-b19483ec606F -s HDDHDDPwd.txt
[2023-05-04 11:42:18.835] Save variable to file: HDDPwd.txt.
[2023-05-04 11:42:18.909] Variable NV+BS '737CDED7-448B-4801-B57D-B19483EC606F:HddPassword' DataSize = 0x48
[2023-05-04 11:42:42.859] Load and set variables from file: HDDPwd.txt.
[2023-05-04 11:42:42.934] Variable NV+BS '737CDED7-448B-4801-B57D-B19483EC606F:HddPassword' DataSize = 0x48
[2023-05-04 11:42:43.082] dmpstore: Failed to set variable HddPassword: Write Protected.


Thanks.

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com> 
Sent: Wednesday, May 3, 2023 9:21 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

That only proves that you did change the interface. But that cannot prove you change it right.

Have you done any function test? For example:
1) The HDD password feature still works?
2) The variable is really locked?


> -----Original Message-----
> From: Liu, Linus <linus.liu@intel.com>
> Sent: Wednesday, May 3, 2023 8:40 AM
> To: Yao, Jiewen <jiewen.yao@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
> 
> 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/PlatformPKProtectionLibV
> > PlatformPKProtectionLib|ar
> > PlatformPKProtectionLib|Po
> > licy/PlatformPKProtectionLibVarPolicy.inf
> >
> >
> > SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariabl
> > SecureBootVariableProvisionLib|eP
> > SecureBootVariableProvisionLib|ro
> > visionLib/SecureBootVariableProvisionLib.inf
> >
> >    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> >
> > +
> > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib
> > VariablePolicyHelperLib|/V
> > VariablePolicyHelperLib|ar
> > iablePolicyHelperLib.inf
> >
> >
> >
> >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >
> >    #
> >
> > --
> > 2.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103946): https://edk2.groups.io/g/devel/message/103946
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy
Posted by Yao, Jiewen 12 months ago
Sounds good. Thank you very much!

Reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>

> -----Original Message-----
> From: Liu, Linus <linus.liu@intel.com>
> Sent: Thursday, May 4, 2023 11:51 AM
> To: Yao, Jiewen <jiewen.yao@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
> 
> Hi Jieewn
> Please refer the below reply.
> 
> Have you done any function test? For example:
> 1) The HDD password feature still works?
>  Linus : yes , HDD password feature still works.
> 
> 2) The variable is really locked?
>  Linus : I've tried using dmpstore command to write HDDPassword in UEFI
> Shell. Can't override it.
> 
> Please refer to the below log.
> [2023-05-04 11:42:11.046] FS1:\> dmpstore -guid 737cded7-448b-4801-
> b57d-b19483ec606F -s HDDHDDPwd.txt
> [2023-05-04 11:42:18.835] Save variable to file: HDDPwd.txt.
> [2023-05-04 11:42:18.909] Variable NV+BS '737CDED7-448B-4801-B57D-
> B19483EC606F:HddPassword' DataSize = 0x48
> [2023-05-04 11:42:42.859] Load and set variables from file: HDDPwd.txt.
> [2023-05-04 11:42:42.934] Variable NV+BS '737CDED7-448B-4801-B57D-
> B19483EC606F:HddPassword' DataSize = 0x48
> [2023-05-04 11:42:43.082] dmpstore: Failed to set variable HddPassword:
> Write Protected.
> 
> 
> Thanks.
> 
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Wednesday, May 3, 2023 9:21 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
> 
> That only proves that you did change the interface. But that cannot prove
> you change it right.
> 
> Have you done any function test? For example:
> 1) The HDD password feature still works?
> 2) The variable is really locked?
> 
> 
> > -----Original Message-----
> > From: Liu, Linus <linus.liu@intel.com>
> > Sent: Wednesday, May 3, 2023 8:40 AM
> > To: Yao, Jiewen <jiewen.yao@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
> >
> > 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/PlatformPKProtectionLibV
> > > PlatformPKProtectionLib|ar
> > > PlatformPKProtectionLib|Po
> > > licy/PlatformPKProtectionLibVarPolicy.inf
> > >
> > >
> > > SecureBootVariableProvisionLib|SecurityPkg/Library/SecureBootVariabl
> > > SecureBootVariableProvisionLib|eP
> > > SecureBootVariableProvisionLib|ro
> > > visionLib/SecureBootVariableProvisionLib.inf
> > >
> > >    TdxLib|MdePkg/Library/TdxLib/TdxLib.inf
> > >
> > > +
> > > VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib
> > > VariablePolicyHelperLib|/V
> > > VariablePolicyHelperLib|ar
> > > iablePolicyHelperLib.inf
> > >
> > >
> > >
> > >  [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >
> > >    #
> > >
> > > --
> > > 2.33.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#104046): https://edk2.groups.io/g/devel/message/104046
Mute This Topic: https://groups.io/mt/98224856/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-