* [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy @ 2023-04-11 9:55 Linus Liu 2023-05-02 16:10 ` Yao, Jiewen 0 siblings, 1 reply; 12+ messages in thread From: Linus Liu @ 2023-04-11 9:55 UTC (permalink / raw) To: devel; +Cc: Jiewen Yao, FST-FIR-PRC, FST FIR Server C, Maggie Chu 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-04-11 9:55 [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy Linus Liu @ 2023-05-02 16:10 ` Yao, Jiewen 2023-05-03 0:39 ` Linus Liu 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2023-05-02 16:10 UTC (permalink / raw) To: Liu, Linus, devel@edk2.groups.io; +Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-02 16:10 ` Yao, Jiewen @ 2023-05-03 0:39 ` Linus Liu 2023-05-03 1:21 ` Yao, Jiewen 0 siblings, 1 reply; 12+ messages in thread From: Linus Liu @ 2023-05-03 0:39 UTC (permalink / raw) To: Yao, Jiewen, devel@edk2.groups.io Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-03 0:39 ` Linus Liu @ 2023-05-03 1:21 ` Yao, Jiewen 2023-05-04 3:50 ` Linus Liu 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2023-05-03 1:21 UTC (permalink / raw) To: Liu, Linus, devel@edk2.groups.io; +Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-03 1:21 ` Yao, Jiewen @ 2023-05-04 3:50 ` Linus Liu 2023-05-04 23:49 ` Yao, Jiewen [not found] ` <175C15AECAAF6F6F.898@groups.io> 0 siblings, 2 replies; 12+ messages in thread From: Linus Liu @ 2023-05-04 3:50 UTC (permalink / raw) To: Yao, Jiewen, devel@edk2.groups.io Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-04 3:50 ` Linus Liu @ 2023-05-04 23:49 ` Yao, Jiewen [not found] ` <175C15AECAAF6F6F.898@groups.io> 1 sibling, 0 replies; 12+ messages in thread From: Yao, Jiewen @ 2023-05-04 23:49 UTC (permalink / raw) To: Liu, Linus, devel@edk2.groups.io; +Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <175C15AECAAF6F6F.898@groups.io>]
* Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy [not found] ` <175C15AECAAF6F6F.898@groups.io> @ 2023-05-05 6:30 ` Yao, Jiewen 2023-05-08 1:08 ` Linus Liu 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2023-05-05 6:30 UTC (permalink / raw) To: devel@edk2.groups.io, Yao, Jiewen, Liu, Linus Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie It seems CI failure - https://github.com/tianocore/edk2/pull/4334 Have you run CI before? > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Friday, May 5, 2023 7:50 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > 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 > > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-05 6:30 ` [edk2-devel] " Yao, Jiewen @ 2023-05-08 1:08 ` Linus Liu 2023-05-08 1:29 ` Yao, Jiewen 0 siblings, 1 reply; 12+ messages in thread From: Linus Liu @ 2023-05-08 1:08 UTC (permalink / raw) To: Yao, Jiewen, devel@edk2.groups.io Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie [-- Attachment #1: Type: text/plain, Size: 12231 bytes --] Hi Jiewen I did. https://github.com/tianocore/edk2/pull/4264 I think you used the previous patch. I've attached the latest patch. Please help to check this . Thanks. -----Original Message----- From: Yao, Jiewen <jiewen.yao@intel.com> Sent: Friday, May 5, 2023 2:30 PM To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Liu, Linus <linus.liu@intel.com> 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy It seems CI failure - https://github.com/tianocore/edk2/pull/4334 Have you run CI before? > -----Original Message----- > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > Jiewen > Sent: Friday, May 5, 2023 7:50 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > 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 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 > > > > > [-- Attachment #2: Type: message/rfc822, Size: 8677 bytes --] From: "Liu, Linus" <linus.liu@intel.com> To: "devel@edk2.groups.io" <devel@edk2.groups.io> Cc: "Liu, Linus" <linus.liu@intel.com>, "Yao, Jiewen" <jiewen.yao@intel.com>, "Chu, Maggie" <maggie.chu@intel.com>, "Kumar, Rahul R" <rahul.r.kumar@intel.com> Subject: [PATCH v1] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy Date: Fri, 28 Apr 2023 02:55:43 +0000 Message-ID: <20230428025543.127-1-linus.liu@intel.com> From: Linus Liu <linus.liu@intel.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408 Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Maggie Chu <maggie.chu@intel.com> Cc: Kumar Rahul <rahul.r.kumar@intel.com> Signed-off-by: Linus Liu <linus.liu@intel.com> --- SecurityPkg/HddPassword/HddPasswordDxe.c | 20 +++++++++++++------- SecurityPkg/HddPassword/HddPasswordDxe.h | 1 - SecurityPkg/HddPassword/HddPasswordDxe.inf | 3 ++- SecurityPkg/SecurityPkg.dsc | 1 + 4 files changed, 16 insertions(+), 9 deletions(-) diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c b/SecurityPkg/HddPassword/HddPasswordDxe.c index 55dfb25886..86c11c749f 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,13 +2859,18 @@ 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, - HDD_PASSWORD_VARIABLE_NAME, - &mHddPasswordVendorGuid - ); + Status = RegisterBasicVariablePolicy ( + VariablePolicy, + &mHddPasswordVendorGuid, + HDD_PASSWORD_VARIABLE_NAME, + 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", __func__, 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.39.2.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-08 1:08 ` Linus Liu @ 2023-05-08 1:29 ` Yao, Jiewen 2023-05-08 5:53 ` Linus Liu 0 siblings, 1 reply; 12+ messages in thread From: Yao, Jiewen @ 2023-05-08 1:29 UTC (permalink / raw) To: Liu, Linus, devel@edk2.groups.io; +Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie https://github.com/tianocore/edk2/pull/4264 is from last month and it is out of date. https://github.com/tianocore/edk2/pull/4334 failed in latest branch. Please try it again. Also, I see you always use [V1] in the patch title. That is very confusing. Please use V2, V3, etc whenever you send a new patch. Thank you Yao, Jiewen > -----Original Message----- > From: Liu, Linus <linus.liu@intel.com> > Sent: Monday, May 8, 2023 9:09 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > Hi Jiewen > I did. > https://github.com/tianocore/edk2/pull/4264 > > I think you used the previous patch. I've attached the latest patch. > Please help to check this . > > Thanks. > > > -----Original Message----- > From: Yao, Jiewen <jiewen.yao@intel.com> > Sent: Friday, May 5, 2023 2:30 PM > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Liu, Linus > <linus.liu@intel.com> > 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > It seems CI failure - https://github.com/tianocore/edk2/pull/4334 > > Have you run CI before? > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > > Jiewen > > Sent: Friday, May 5, 2023 7:50 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > > HddPasswordDxeInit to use Variable Policy > > > > 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 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 > > > > > > > > > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy 2023-05-08 1:29 ` Yao, Jiewen @ 2023-05-08 5:53 ` Linus Liu 0 siblings, 0 replies; 12+ messages in thread From: Linus Liu @ 2023-05-08 5:53 UTC (permalink / raw) To: Yao, Jiewen, devel@edk2.groups.io Cc: FST-FIR-PRC, FST FIR Server, Chu, Maggie [-- Attachment #1: Type: text/plain, Size: 14245 bytes --] Hi Jiewen I've resent the patch and verify the patch. https://github.com/tianocore/edk2/pull/4354 Thanks. -----Original Message----- From: Yao, Jiewen <jiewen.yao@intel.com> Sent: Monday, May 8, 2023 9:30 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy https://github.com/tianocore/edk2/pull/4264 is from last month and it is out of date. https://github.com/tianocore/edk2/pull/4334 failed in latest branch. Please try it again. Also, I see you always use [V1] in the patch title. That is very confusing. Please use V2, V3, etc whenever you send a new patch. Thank you Yao, Jiewen > -----Original Message----- > From: Liu, Linus <linus.liu@intel.com> > Sent: Monday, May 8, 2023 9:09 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > Hi Jiewen > I did. > https://github.com/tianocore/edk2/pull/4264 > > I think you used the previous patch. I've attached the latest patch. > Please help to check this . > > Thanks. > > > -----Original Message----- > From: Yao, Jiewen <jiewen.yao@intel.com> > Sent: Friday, May 5, 2023 2:30 PM > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Liu, > Linus <linus.liu@intel.com> > 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > HddPasswordDxeInit to use Variable Policy > > It seems CI failure - https://github.com/tianocore/edk2/pull/4334 > > Have you run CI before? > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > > Jiewen > > Sent: Friday, May 5, 2023 7:50 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: [edk2-devel] [PATCH] Securitypkg/hddpassword: Update > > HddPasswordDxeInit to use Variable Policy > > > > 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 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 > > > > > > > > > > [-- Attachment #2: Type: message/rfc822, Size: 9020 bytes --] From: "Liu, Linus" <linus.liu@intel.com> To: "devel@edk2.group.io" <devel@edk2.group.io> Cc: "Liu, Linus" <linus.liu@intel.com>, "Yao, Jiewen" <jiewen.yao@intel.com>, "Chu, Maggie" <maggie.chu@intel.com>, "Kumar, Rahul R" <rahul.r.kumar@intel.com> Subject: [PATCH v4] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy Date: Mon, 8 May 2023 04:20:51 +0000 Message-ID: <20230508042051.1934-1-linus.liu@intel.com> From: Linus Liu <linus.liu@intel.com> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408 Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Maggie Chu <maggie.chu@intel.com> Cc: Kumar Rahul <rahul.r.kumar@intel.com> Signed-off-by: Linus Liu <linus.liu@intel.com> --- SecurityPkg/HddPassword/HddPasswordDxe.c | 28 ++++++++++++-------- SecurityPkg/HddPassword/HddPasswordDxe.h | 1 - SecurityPkg/HddPassword/HddPasswordDxe.inf | 3 ++- SecurityPkg/SecurityPkg.dsc | 1 + 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/SecurityPkg/HddPassword/HddPasswordDxe.c b/SecurityPkg/HddPassword/HddPasswordDxe.c index 55dfb25886..6f36b5a0a2 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"; @@ -2818,11 +2819,11 @@ HddPasswordDxeInit ( IN EFI_SYSTEM_TABLE *SystemTable ) { - EFI_STATUS Status; - HDD_PASSWORD_DXE_PRIVATE_DATA *Private; - VOID *Registration; - EFI_EVENT EndOfDxeEvent; - EDKII_VARIABLE_LOCK_PROTOCOL *VariableLock; + EFI_STATUS Status; + HDD_PASSWORD_DXE_PRIVATE_DATA *Private; + VOID *Registration; + EFI_EVENT EndOfDxeEvent; + EDKII_VARIABLE_POLICY_PROTOCOL *VariablePolicy; Private = NULL; @@ -2858,13 +2859,18 @@ 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, - HDD_PASSWORD_VARIABLE_NAME, - &mHddPasswordVendorGuid - ); + Status = RegisterBasicVariablePolicy ( + VariablePolicy, + &mHddPasswordVendorGuid, + HDD_PASSWORD_VARIABLE_NAME, + 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", __func__, 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.39.2.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy @ 2023-04-11 3:53 Linus Liu 0 siblings, 0 replies; 12+ messages in thread From: Linus Liu @ 2023-04-11 3:53 UTC (permalink / raw) To: devel; +Cc: Jiewen Yao, Jian J Wang, Maggie Chu REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408 Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c Cc: Jiewen Yao <jiewen.yao@intel.com> Cc: Jian J Wang <jian.j.wang@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 ++- 3 files changed, 13 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 -- 2.33.1.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy @ 2023-04-11 3:47 linus.liu 0 siblings, 0 replies; 12+ messages in thread From: linus.liu @ 2023-04-11 3:47 UTC (permalink / raw) To: devel; +Cc: Jiewen Yao REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4408 Change-Id: I3c4b466ef318766d6d70c9f73e36b94b5f10832c Cc: Jiewen Yao <jiewen.yao@intel.com>, Jian J Wang <jian.j.wang@intel.com>, 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 ++- 3 files changed, 13 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 -- 2.33.1.windows.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
end of thread, other threads:[~2023-05-08 5:53 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-11 9:55 [PATCH] Securitypkg/hddpassword: Update HddPasswordDxeInit to use Variable Policy Linus Liu
2023-05-02 16:10 ` Yao, Jiewen
2023-05-03 0:39 ` Linus Liu
2023-05-03 1:21 ` Yao, Jiewen
2023-05-04 3:50 ` Linus Liu
2023-05-04 23:49 ` Yao, Jiewen
[not found] ` <175C15AECAAF6F6F.898@groups.io>
2023-05-05 6:30 ` [edk2-devel] " Yao, Jiewen
2023-05-08 1:08 ` Linus Liu
2023-05-08 1:29 ` Yao, Jiewen
2023-05-08 5:53 ` Linus Liu
-- strict thread matches above, loose matches on Subject: below --
2023-04-11 3:53 Linus Liu
2023-04-11 3:47 linus.liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox