public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec
@ 2024-04-02  8:32 Tan, Ming
  2024-04-04 15:21 ` Felix Polyudov via groups.io
  2024-04-07  2:07 ` Dandan Bi
  0 siblings, 2 replies; 4+ messages in thread
From: Tan, Ming @ 2024-04-02  8:32 UTC (permalink / raw)
  To: devel; +Cc: Min Xu, Jiewen Yao, Dandan Bi, Felix Polyudov

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713

In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
EFI_BROWSER_ACTION_FORM_OPEN:
NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with
this browser action because question values have not been retrieved yet.

So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
call back function.

Now call SecureBootExtractConfigFromVariable() and update
IfrNvData->ListCount to save the change to EFI variable, then HII use EFI
variable to control the UI.

Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Felix Polyudov <Felixp@ami.com>
Signed-off-by: Ming Tan <ming.tan@intel.com>
---
  PR: https://github.com/tianocore/edk2/pull/5411

  V4: Fix a Cc issue of miss a space.
  V3: According to Dandan Bi's feedback, does not call SecureBootExtractConfigFromVariable() at last, but call it as needed.
      And add more code for update IfrNvData->ListCount.
  V2: Change code style to pass uncrustify check.

 .../SecureBootConfigImpl.c                    | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 2c11129526..6d4560c39b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
     ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;
   }
 
+  ConfigData->ListCount = Private->ListCount;
+
   //
   // If it is Physical Presence User, set the PhysicalPresent to true.
   //
@@ -4541,12 +4543,13 @@ SecureBootCallback (
   EFI_HII_POPUP_PROTOCOL          *HiiPopup;
   EFI_HII_POPUP_SELECTION         UserSelection;
 
-  Status             = EFI_SUCCESS;
-  SecureBootEnable   = NULL;
-  SecureBootMode     = NULL;
-  SetupMode          = NULL;
-  File               = NULL;
-  EnrollKeyErrorCode = None_Error;
+  Status               = EFI_SUCCESS;
+  SecureBootEnable     = NULL;
+  SecureBootMode       = NULL;
+  SetupMode            = NULL;
+  File                 = NULL;
+  EnrollKeyErrorCode   = None_Error;
+  GetBrowserDataResult = FALSE;
 
   if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {
     return EFI_INVALID_PARAMETER;
@@ -4565,15 +4568,12 @@ SecureBootCallback (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);
-
   if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {
     if (QuestionId == KEY_SECURE_BOOT_MODE) {
       //
       // Update secure boot strings when opening this form
       //
-      Status = UpdateSecureBootString (Private);
-      SecureBootExtractConfigFromVariable (Private, IfrNvData);
+      Status                 = UpdateSecureBootString (Private);
       mIsEnterSecureBootForm = TRUE;
     } else {
       //
@@ -4587,23 +4587,22 @@ SecureBootCallback (
           (QuestionId == KEY_SECURE_BOOT_DBT_OPTION))
       {
         CloseEnrolledFile (Private->FileContext);
-      } else if (QuestionId == KEY_SECURE_BOOT_DELETE_ALL_LIST) {
-        //
-        // Update ListCount field in varstore
-        // Button "Delete All Signature List" is
-        // enable when ListCount is greater than 0.
-        //
-        IfrNvData->ListCount = Private->ListCount;
       }
     }
 
     goto EXIT;
   }
 
+  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);
+
   if (Action == EFI_BROWSER_ACTION_RETRIEVE) {
     Status = EFI_UNSUPPORTED;
     if (QuestionId == KEY_SECURE_BOOT_MODE) {
       if (mIsEnterSecureBootForm) {
+        if (GetBrowserDataResult) {
+          SecureBootExtractConfigFromVariable (Private, IfrNvData);
+        }
+
         Value->u8 = SECURE_BOOT_MODE_STANDARD;
         Status    = EFI_SUCCESS;
       }
@@ -4764,6 +4763,8 @@ SecureBootCallback (
                 L"Only Physical Presence User could delete PK in custom mode!",
                 NULL
                 );
+            } else {
+              SecureBootExtractConfigFromVariable (Private, IfrNvData);
             }
           }
         }
@@ -4827,6 +4828,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
           OPTION_SIGNATURE_LIST_QUESTION_ID
           );
+        IfrNvData->ListCount = Private->ListCount;
         break;
 
       //
@@ -4851,6 +4853,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
           OPTION_SIGNATURE_LIST_QUESTION_ID
           );
+        IfrNvData->ListCount = Private->ListCount;
         break;
 
       //
@@ -4875,6 +4878,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
           OPTION_SIGNATURE_LIST_QUESTION_ID
           );
+        IfrNvData->ListCount = Private->ListCount;
         break;
 
       case SECUREBOOT_DELETE_SIGNATURE_FROM_DBT:
@@ -4954,6 +4958,8 @@ SecureBootCallback (
             L"Only supports DER-encoded X509 certificate, AUTH_2 format data & executable EFI image",
             NULL
             );
+        } else {
+          IfrNvData->ListCount = Private->ListCount;
         }
 
         break;
@@ -5005,6 +5011,8 @@ SecureBootCallback (
             PromptString,
             NULL
             );
+        } else {
+          SecureBootExtractConfigFromVariable (Private, IfrNvData);
         }
 
         break;
-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117332): https://edk2.groups.io/g/devel/message/117332
Mute This Topic: https://groups.io/mt/105284072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec
  2024-04-02  8:32 [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec Tan, Ming
@ 2024-04-04 15:21 ` Felix Polyudov via groups.io
  2024-04-07  2:07 ` Dandan Bi
  1 sibling, 0 replies; 4+ messages in thread
From: Felix Polyudov via groups.io @ 2024-04-04 15:21 UTC (permalink / raw)
  To: Ming Tan, devel; +Cc: Min Xu, Jiewen Yao, Dandan Bi

Reviewed-by: Felix Polyudov <felixp@...>

-----Original Message-----
From: Ming Tan <ming.tan@intel.com>
Sent: Tuesday, April 2, 2024 4:32 AM
To: devel@edk2.groups.io
Cc: Min Xu <min.m.xu@intel.com>; Jiewen Yao <jiewen.yao@intel.com>; Dandan Bi <dandan.bi@intel.com>; Felix Polyudov <Felixp@ami.com>
Subject: [EXTERNAL] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec


**CAUTION: The e-mail below is from an external source. Please exercise caution before opening attachments, clicking links, or following guidance.**

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713

In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
EFI_BROWSER_ACTION_FORM_OPEN:
NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with this browser action because question values have not been retrieved yet.

So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN call back function.

Now call SecureBootExtractConfigFromVariable() and update
IfrNvData->ListCount to save the change to EFI variable, then HII use
IfrNvData->EFI
variable to control the UI.

Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Felix Polyudov <Felixp@ami.com>
Signed-off-by: Ming Tan <ming.tan@intel.com>
---
  PR: https://github.com/tianocore/edk2/pull/5411

  V4: Fix a Cc issue of miss a space.
  V3: According to Dandan Bi's feedback, does not call SecureBootExtractConfigFromVariable() at last, but call it as needed.
      And add more code for update IfrNvData->ListCount.
  V2: Change code style to pass uncrustify check.

 .../SecureBootConfigImpl.c                    | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 2c11129526..6d4560c39b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.c
@@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
     ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;

   }



+  ConfigData->ListCount = Private->ListCount;

+

   //

   // If it is Physical Presence User, set the PhysicalPresent to true.

   //

@@ -4541,12 +4543,13 @@ SecureBootCallback (
   EFI_HII_POPUP_PROTOCOL          *HiiPopup;

   EFI_HII_POPUP_SELECTION         UserSelection;



-  Status             = EFI_SUCCESS;

-  SecureBootEnable   = NULL;

-  SecureBootMode     = NULL;

-  SetupMode          = NULL;

-  File               = NULL;

-  EnrollKeyErrorCode = None_Error;

+  Status               = EFI_SUCCESS;

+  SecureBootEnable     = NULL;

+  SecureBootMode       = NULL;

+  SetupMode            = NULL;

+  File                 = NULL;

+  EnrollKeyErrorCode   = None_Error;

+  GetBrowserDataResult = FALSE;



   if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {

     return EFI_INVALID_PARAMETER;

@@ -4565,15 +4568,12 @@ SecureBootCallback (
     return EFI_OUT_OF_RESOURCES;

   }



-  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);

-

   if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {

     if (QuestionId == KEY_SECURE_BOOT_MODE) {

       //

       // Update secure boot strings when opening this form

       //

-      Status = UpdateSecureBootString (Private);

-      SecureBootExtractConfigFromVariable (Private, IfrNvData);

+      Status                 = UpdateSecureBootString (Private);

       mIsEnterSecureBootForm = TRUE;

     } else {

       //

@@ -4587,23 +4587,22 @@ SecureBootCallback (
           (QuestionId == KEY_SECURE_BOOT_DBT_OPTION))

       {

         CloseEnrolledFile (Private->FileContext);

-      } else if (QuestionId == KEY_SECURE_BOOT_DELETE_ALL_LIST) {

-        //

-        // Update ListCount field in varstore

-        // Button "Delete All Signature List" is

-        // enable when ListCount is greater than 0.

-        //

-        IfrNvData->ListCount = Private->ListCount;

       }

     }



     goto EXIT;

   }



+  GetBrowserDataResult = HiiGetBrowserData
+ (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize,
+ (UINT8 *)IfrNvData);

+

   if (Action == EFI_BROWSER_ACTION_RETRIEVE) {

     Status = EFI_UNSUPPORTED;

     if (QuestionId == KEY_SECURE_BOOT_MODE) {

       if (mIsEnterSecureBootForm) {

+        if (GetBrowserDataResult) {

+          SecureBootExtractConfigFromVariable (Private, IfrNvData);

+        }

+

         Value->u8 = SECURE_BOOT_MODE_STANDARD;

         Status    = EFI_SUCCESS;

       }

@@ -4764,6 +4763,8 @@ SecureBootCallback (
                 L"Only Physical Presence User could delete PK in custom mode!",

                 NULL

                 );

+            } else {

+              SecureBootExtractConfigFromVariable (Private, IfrNvData);

             }

           }

         }

@@ -4827,6 +4828,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,

           OPTION_SIGNATURE_LIST_QUESTION_ID

           );

+        IfrNvData->ListCount = Private->ListCount;

         break;



       //

@@ -4851,6 +4853,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,

           OPTION_SIGNATURE_LIST_QUESTION_ID

           );

+        IfrNvData->ListCount = Private->ListCount;

         break;



       //

@@ -4875,6 +4878,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,

           OPTION_SIGNATURE_LIST_QUESTION_ID

           );

+        IfrNvData->ListCount = Private->ListCount;

         break;



       case SECUREBOOT_DELETE_SIGNATURE_FROM_DBT:

@@ -4954,6 +4958,8 @@ SecureBootCallback (
             L"Only supports DER-encoded X509 certificate, AUTH_2 format data & executable EFI image",

             NULL

             );

+        } else {

+          IfrNvData->ListCount = Private->ListCount;

         }



         break;

@@ -5005,6 +5011,8 @@ SecureBootCallback (
             PromptString,

             NULL

             );

+        } else {

+          SecureBootExtractConfigFromVariable (Private, IfrNvData);

         }



         break;

--
2.31.1.windows.1

-The information contained in this message may be confidential and proprietary to American Megatrends (AMI). This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117439): https://edk2.groups.io/g/devel/message/117439
Mute This Topic: https://groups.io/mt/105284072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec
  2024-04-02  8:32 [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec Tan, Ming
  2024-04-04 15:21 ` Felix Polyudov via groups.io
@ 2024-04-07  2:07 ` Dandan Bi
  2024-04-07  3:52   ` Yao, Jiewen
  1 sibling, 1 reply; 4+ messages in thread
From: Dandan Bi @ 2024-04-07  2:07 UTC (permalink / raw)
  To: Tan, Ming, devel; +Cc: Xu, Min M, Yao, Jiewen, POLUDOV, FELIX

Reviewed-by: Dandan Bi <dandan.bi@intel.com>

-----Original Message-----
From: Tan, Ming <ming.tan@intel.com> 
Sent: Tuesday, April 2, 2024 4:32 PM
To: devel@edk2.groups.io
Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Bi, Dandan <dandan.bi@intel.com>; POLUDOV, FELIX <felixp@ami.com>
Subject: [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713

In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
EFI_BROWSER_ACTION_FORM_OPEN:
NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used with this browser action because question values have not been retrieved yet.

So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN call back function.

Now call SecureBootExtractConfigFromVariable() and update
IfrNvData->ListCount to save the change to EFI variable, then HII use 
IfrNvData->EFI
variable to control the UI.

Cc: Min Xu <min.m.xu@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Dandan Bi <dandan.bi@intel.com>
Cc: Felix Polyudov <Felixp@ami.com>
Signed-off-by: Ming Tan <ming.tan@intel.com>
---
  PR: https://github.com/tianocore/edk2/pull/5411

  V4: Fix a Cc issue of miss a space.
  V3: According to Dandan Bi's feedback, does not call SecureBootExtractConfigFromVariable() at last, but call it as needed.
      And add more code for update IfrNvData->ListCount.
  V2: Change code style to pass uncrustify check.

 .../SecureBootConfigImpl.c                    | 42 +++++++++++--------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index 2c11129526..6d4560c39b 100644
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
+++ nfigImpl.c
@@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
     ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;   } +  ConfigData->ListCount = Private->ListCount;+   //   // If it is Physical Presence User, set the PhysicalPresent to true.   //@@ -4541,12 +4543,13 @@ SecureBootCallback (
   EFI_HII_POPUP_PROTOCOL          *HiiPopup;   EFI_HII_POPUP_SELECTION         UserSelection; -  Status             = EFI_SUCCESS;-  SecureBootEnable   = NULL;-  SecureBootMode     = NULL;-  SetupMode          = NULL;-  File               = NULL;-  EnrollKeyErrorCode = None_Error;+  Status               = EFI_SUCCESS;+  SecureBootEnable     = NULL;+  SecureBootMode       = NULL;+  SetupMode            = NULL;+  File                 = NULL;+  EnrollKeyErrorCode   = None_Error;+  GetBrowserDataResult = FALSE;    if ((This == NULL) || (Value == NULL) || (ActionRequest == NULL)) {     return EFI_INVALID_PARAMETER;@@ -4565,15 +4568,12 @@ SecureBootCallback (
     return EFI_OUT_OF_RESOURCES;   } -  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);-   if (Action == EFI_BROWSER_ACTION_FORM_OPEN) {     if (QuestionId == KEY_SECURE_BOOT_MODE) {       //       // Update secure boot strings when opening this form       //-      Status = UpdateSecureBootString (Private);-      SecureBootExtractConfigFromVariable (Private, IfrNvData);+      Status                 = UpdateSecureBootString (Private);       mIsEnterSecureBootForm = TRUE;     } else {       //@@ -4587,23 +4587,22 @@ SecureBootCallback (
           (QuestionId == KEY_SECURE_BOOT_DBT_OPTION))       {         CloseEnrolledFile (Private->FileContext);-      } else if (QuestionId == KEY_SECURE_BOOT_DELETE_ALL_LIST) {-        //-        // Update ListCount field in varstore-        // Button "Delete All Signature List" is-        // enable when ListCount is greater than 0.-        //-        IfrNvData->ListCount = Private->ListCount;       }     }      goto EXIT;   } +  GetBrowserDataResult = HiiGetBrowserData (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);+   if (Action == EFI_BROWSER_ACTION_RETRIEVE) {     Status = EFI_UNSUPPORTED;     if (QuestionId == KEY_SECURE_BOOT_MODE) {       if (mIsEnterSecureBootForm) {+        if (GetBrowserDataResult) {+          SecureBootExtractConfigFromVariable (Private, IfrNvData);+        }+         Value->u8 = SECURE_BOOT_MODE_STANDARD;         Status    = EFI_SUCCESS;       }@@ -4764,6 +4763,8 @@ SecureBootCallback (
                 L"Only Physical Presence User could delete PK in custom mode!",                 NULL                 );+            } else {+              SecureBootExtractConfigFromVariable (Private, IfrNvData);             }           }         }@@ -4827,6 +4828,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,           OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount = Private->ListCount;         break;        //@@ -4851,6 +4853,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,           OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount = Private->ListCount;         break;        //@@ -4875,6 +4878,7 @@ SecureBootCallback (
           SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,           OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount = Private->ListCount;         break;        case SECUREBOOT_DELETE_SIGNATURE_FROM_DBT:@@ -4954,6 +4958,8 @@ SecureBootCallback (
             L"Only supports DER-encoded X509 certificate, AUTH_2 format data & executable EFI image",             NULL             );+        } else {+          IfrNvData->ListCount = Private->ListCount;         }          break;@@ -5005,6 +5011,8 @@ SecureBootCallback (
             PromptString,             NULL             );+        } else {+          SecureBootExtractConfigFromVariable (Private, IfrNvData);         }          break;-- 
2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117472): https://edk2.groups.io/g/devel/message/117472
Mute This Topic: https://groups.io/mt/105284072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec
  2024-04-07  2:07 ` Dandan Bi
@ 2024-04-07  3:52   ` Yao, Jiewen
  0 siblings, 0 replies; 4+ messages in thread
From: Yao, Jiewen @ 2024-04-07  3:52 UTC (permalink / raw)
  To: Bi, Dandan, Tan, Ming, devel; +Cc: Xu, Min M, POLUDOV, FELIX

Thanks.https://github.com/tianocore/edk2/pull/5533

> -----Original Message-----
> From: Bi, Dandan <dandan.bi@intel.com>
> Sent: Sunday, April 7, 2024 10:07 AM
> To: Tan, Ming <ming.tan@intel.com>; devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> POLUDOV, FELIX <felixp@ami.com>
> Subject: RE: [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according
> to UEFI spec
> 
> Reviewed-by: Dandan Bi <dandan.bi@intel.com>
> 
> -----Original Message-----
> From: Tan, Ming <ming.tan@intel.com>
> Sent: Tuesday, April 2, 2024 4:32 PM
> To: devel@edk2.groups.io
> Cc: Xu, Min M <min.m.xu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Bi,
> Dandan <dandan.bi@intel.com>; POLUDOV, FELIX <felixp@ami.com>
> Subject: [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to
> UEFI spec
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4713
> 
> In UEFI_Spec_2_10_Aug29.pdf page 1694 section 35.5.4 for
> EFI_BROWSER_ACTION_FORM_OPEN:
> NOTE: EFI_FORM_BROWSER2_PROTOCOL.BrowserCallback() cannot be used
> with this browser action because question values have not been retrieved yet.
> 
> So should not call HiiGetBrowserData() and HiiSetBrowserData() in FORM_OPEN
> call back function.
> 
> Now call SecureBootExtractConfigFromVariable() and update
> IfrNvData->ListCount to save the change to EFI variable, then HII use
> IfrNvData->EFI
> variable to control the UI.
> 
> Cc: Min Xu <min.m.xu@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Dandan Bi <dandan.bi@intel.com>
> Cc: Felix Polyudov <Felixp@ami.com>
> Signed-off-by: Ming Tan <ming.tan@intel.com>
> ---
>   PR: https://github.com/tianocore/edk2/pull/5411
> 
>   V4: Fix a Cc issue of miss a space.
>   V3: According to Dandan Bi's feedback, does not call
> SecureBootExtractConfigFromVariable() at last, but call it as needed.
>       And add more code for update IfrNvData->ListCount.
>   V2: Change code style to pass uncrustify check.
> 
>  .../SecureBootConfigImpl.c                    | 42 +++++++++++--------
>  1 file changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> index 2c11129526..6d4560c39b 100644
> ---
> a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigIm
> pl.c
> +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootCo
> +++ nfigImpl.c
> @@ -3366,6 +3366,8 @@ SecureBootExtractConfigFromVariable (
>      ConfigData->FileEnrollType = UNKNOWN_FILE_TYPE;   } +  ConfigData-
> >ListCount = Private->ListCount;+   //   // If it is Physical Presence User, set the
> PhysicalPresent to true.   //@@ -4541,12 +4543,13 @@ SecureBootCallback (
>    EFI_HII_POPUP_PROTOCOL          *HiiPopup;   EFI_HII_POPUP_SELECTION
> UserSelection; -  Status             = EFI_SUCCESS;-  SecureBootEnable   = NULL;-
> SecureBootMode     = NULL;-  SetupMode          = NULL;-  File               = NULL;-
> EnrollKeyErrorCode = None_Error;+  Status               = EFI_SUCCESS;+
> SecureBootEnable     = NULL;+  SecureBootMode       = NULL;+  SetupMode
> = NULL;+  File                 = NULL;+  EnrollKeyErrorCode   = None_Error;+
> GetBrowserDataResult = FALSE;    if ((This == NULL) || (Value == NULL) ||
> (ActionRequest == NULL)) {     return EFI_INVALID_PARAMETER;@@ -4565,15
> +4568,12 @@ SecureBootCallback (
>      return EFI_OUT_OF_RESOURCES;   } -  GetBrowserDataResult =
> HiiGetBrowserData (&gSecureBootConfigFormSetGuid,
> mSecureBootStorageName, BufferSize, (UINT8 *)IfrNvData);-   if (Action ==
> EFI_BROWSER_ACTION_FORM_OPEN) {     if (QuestionId ==
> KEY_SECURE_BOOT_MODE) {       //       // Update secure boot strings when
> opening this form       //-      Status = UpdateSecureBootString (Private);-
> SecureBootExtractConfigFromVariable (Private, IfrNvData);+      Status                 =
> UpdateSecureBootString (Private);       mIsEnterSecureBootForm = TRUE;     } else
> {       //@@ -4587,23 +4587,22 @@ SecureBootCallback (
>            (QuestionId == KEY_SECURE_BOOT_DBT_OPTION))
> {         CloseEnrolledFile (Private->FileContext);-      } else if (QuestionId ==
> KEY_SECURE_BOOT_DELETE_ALL_LIST) {-        //-        // Update ListCount field in
> varstore-        // Button "Delete All Signature List" is-        // enable when ListCount
> is greater than 0.-        //-        IfrNvData->ListCount = Private->ListCount;       }     }
> goto EXIT;   } +  GetBrowserDataResult = HiiGetBrowserData
> (&gSecureBootConfigFormSetGuid, mSecureBootStorageName, BufferSize,
> (UINT8 *)IfrNvData);+   if (Action == EFI_BROWSER_ACTION_RETRIEVE)
> {     Status = EFI_UNSUPPORTED;     if (QuestionId == KEY_SECURE_BOOT_MODE)
> {       if (mIsEnterSecureBootForm) {+        if (GetBrowserDataResult) {+
> SecureBootExtractConfigFromVariable (Private, IfrNvData);+        }+         Value-
> >u8 = SECURE_BOOT_MODE_STANDARD;         Status    = EFI_SUCCESS;       }@@ -
> 4764,6 +4763,8 @@ SecureBootCallback (
>                  L"Only Physical Presence User could delete PK in custom mode!",
> NULL                 );+            } else {+              SecureBootExtractConfigFromVariable
> (Private, IfrNvData);             }           }         }@@ -4827,6 +4828,7 @@
> SecureBootCallback (
>            SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
> OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount =
> Private->ListCount;         break;        //@@ -4851,6 +4853,7 @@
> SecureBootCallback (
>            SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
> OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount =
> Private->ListCount;         break;        //@@ -4875,6 +4878,7 @@
> SecureBootCallback (
>            SECUREBOOT_DELETE_SIGNATURE_LIST_FORM,
> OPTION_SIGNATURE_LIST_QUESTION_ID           );+        IfrNvData->ListCount =
> Private->ListCount;         break;        case
> SECUREBOOT_DELETE_SIGNATURE_FROM_DBT:@@ -4954,6 +4958,8 @@
> SecureBootCallback (
>              L"Only supports DER-encoded X509 certificate, AUTH_2 format data &
> executable EFI image",             NULL             );+        } else {+          IfrNvData-
> >ListCount = Private->ListCount;         }          break;@@ -5005,6 +5011,8 @@
> SecureBootCallback (
>              PromptString,             NULL             );+        } else {+
> SecureBootExtractConfigFromVariable (Private, IfrNvData);         }          break;--
> 2.31.1.windows.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117473): https://edk2.groups.io/g/devel/message/117473
Mute This Topic: https://groups.io/mt/105284072/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-04-07  3:52 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-02  8:32 [edk2-devel] [PATCH v4] SecurityPkg/SecureBootConfigDxe: Update UI according to UEFI spec Tan, Ming
2024-04-04 15:21 ` Felix Polyudov via groups.io
2024-04-07  2:07 ` Dandan Bi
2024-04-07  3:52   ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox