SecurityPkg: AuthVariableLib & SecureBootConfigDxe: Fix SecureBootEnable & PK inconsi...

Revert previous fix in AuthVariable driver init which breaks SecureBootEnable original behavior. Add more error handling logic in SecureBootConfigDxe to prevent wrong display info when SecureBootEnable & PK inconsistency happens.
Commit hash for the reverted patch in AuthVariable driver is
SHA-1: a6811666b0bef18871fa62b6c5abf18fb076fd0d

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Chao Zhang <chao.b.zhang@intel.com>
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>

diff --git a/SecurityPkg/Library/AuthVariableLib/AuthService.c b/SecurityPkg/Library/AuthVariableLib/AuthService.c
index f11b86827a8569e030e48e49ef29a73e902bf4b5..4649e50e5e083584532d047a9ec7c002f3d22eab 100644 (file)
--- a/SecurityPkg/Library/AuthVariableLib/AuthService.c
+++ b/SecurityPkg/Library/AuthVariableLib/AuthService.c
@@ -441,19 +441,7 @@ InitSecureBootVariables (
   SecureBootEnable = SECURE_BOOT_DISABLE;\r
   Status = AuthServiceInternalFindVariable (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID **)&Data, &DataSize);\r
   if (!EFI_ERROR(Status)) {\r
-    if (!IsPkPresent) {\r
-      //\r
-      // PK is cleared in runtime. "SecureBootMode" is not updated before reboot\r
-      // Delete "SecureBootMode"\r
-      //\r
-      Status = AuthServiceInternalUpdateVariable (\r
-                 EFI_SECURE_BOOT_ENABLE_NAME,\r
-                 &gEfiSecureBootEnableDisableGuid,\r
-                 &SecureBootEnable,\r
-                 0,\r
-                 EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS\r
-                 );\r
-    } else {\r
+    if (IsPkPresent) {\r
       SecureBootEnable = *Data;\r
     }\r
   } else if ((SecureBootMode == SecureBootModeTypeUserMode) || (SecureBootMode == SecureBootModeTypeDeployedMode)) {\r

diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
index e840316368919a4b75760769554d9da68dfb2a7d..c8f4d977d92a5506d7a4cdd2c556276d9a5087ae 100644 (file)
--- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
+++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c
@@ -3167,20 +3167,6 @@ SecureBootExtractConfigFromVariable (
   ConfigData->RevocationTime.Minute = CurrTime.Minute;\r
   ConfigData->RevocationTime.Second = 0;\r
 \r
-  //\r
-  // If the SecureBootEnable Variable doesn't exist, hide the SecureBoot Enable/Disable\r
-  // Checkbox.\r
-  //\r
-  ConfigData->AttemptSecureBoot = FALSE;\r
-  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);\r
-  if (SecureBootEnable == NULL) {\r
-    ConfigData->HideSecureBoot = TRUE;\r
-  } else {\r
-    ConfigData->HideSecureBoot = FALSE;\r
-    if ((*SecureBootEnable) == SECURE_BOOT_ENABLE) {\r
-      ConfigData->AttemptSecureBoot = TRUE;\r
-    }\r
-  }\r
 \r
   //\r
   // If it is Physical Presence User, set the PhysicalPresent to true.\r
@@ -3215,6 +3201,26 @@ SecureBootExtractConfigFromVariable (
     ConfigData->HasPk = TRUE;\r
   }\r
 \r
+  //\r
+  // Check SecureBootEnable & Pk status, fix the inconsistence. \r
+  // If the SecureBootEnable Variable doesn't exist, hide the SecureBoot Enable/Disable\r
+  // Checkbox.\r
+  //\r
+  ConfigData->AttemptSecureBoot = FALSE;\r
+  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);  \r
+\r
+  //\r
+  // Fix Pk, SecureBootEnable inconsistence\r
+  //\r
+  if (ConfigData->CurSecureBootMode == SECURE_BOOT_MODE_USER_MODE || ConfigData->CurSecureBootMode == SECURE_BOOT_MODE_DEPLOYED_MODE) {\r
+    ConfigData->HideSecureBoot = FALSE;\r
+    if ((SecureBootEnable != NULL) && (*SecureBootEnable == SECURE_BOOT_ENABLE)) {\r
+      ConfigData->AttemptSecureBoot = TRUE;\r
+    }\r
+  } else {\r
+    ConfigData->HideSecureBoot = TRUE;\r
+  }\r
+\r
   if (SecureBootEnable != NULL) {\r
     FreePool (SecureBootEnable);\r
   }\r
@@ -3363,7 +3369,6 @@ SecureBootRouteConfig (
        OUT EFI_STRING                          *Progress\r
   )\r
 {\r
-  UINT8                      *SecureBootEnable;\r
   SECUREBOOT_CONFIGURATION   IfrNvData;\r
   UINTN                      BufferSize;\r
   EFI_STATUS                 Status;\r
@@ -3400,10 +3405,7 @@ SecureBootRouteConfig (
   //\r
   // Store Buffer Storage back to EFI variable if needed\r
   //\r
-  SecureBootEnable = NULL;\r
-  GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);\r
-  if (NULL != SecureBootEnable) {\r
-    FreePool (SecureBootEnable);\r
+  if (!IfrNvData.HideSecureBoot) {\r
     Status = SaveSecureBootVariable (IfrNvData.AttemptSecureBoot);\r
     if (EFI_ERROR (Status)) {\r
       return Status;\r
@@ -3454,6 +3456,7 @@ SecureBootCallback (
   SECUREBOOT_CONFIGURATION        *IfrNvData;\r
   UINT16                          LabelId;\r
   UINT8                           *SecureBootEnable;\r
+  UINT8                           *Pk;\r
   UINT8                           *SecureBootMode;\r
   CHAR16                          PromptString[100];\r
   UINT8                           CurSecureBootMode;\r
@@ -3926,11 +3929,11 @@ SecureBootCallback (
     }\r
   } else if (Action == EFI_BROWSER_ACTION_DEFAULT_STANDARD) {\r
     if (QuestionId == KEY_HIDE_SECURE_BOOT) {\r
-      GetVariable2 (EFI_SECURE_BOOT_ENABLE_NAME, &gEfiSecureBootEnableDisableGuid, (VOID**)&SecureBootEnable, NULL);\r
-      if (SecureBootEnable == NULL) {\r
+      GetVariable2 (EFI_PLATFORM_KEY_NAME, &gEfiGlobalVariableGuid, (VOID**)&Pk, NULL);\r
+      if (Pk == NULL) {\r
         IfrNvData->HideSecureBoot = TRUE;\r
       } else {\r
-        FreePool (SecureBootEnable);\r
+        FreePool (Pk);\r
         IfrNvData->HideSecureBoot = FALSE;\r
       }\r
       Value->b = IfrNvData->HideSecureBoot;\r