From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f51.google.com (mail-wr1-f51.google.com [209.85.221.51]) by mx.groups.io with SMTP id smtpd.web12.1304.1622655620643869749 for ; Wed, 02 Jun 2021 10:40:21 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@akeo-ie.20150623.gappssmtp.com header.s=20150623 header.b=bXMZd/95; spf=pass (domain: akeo.ie, ip: 209.85.221.51, mailfrom: pete@akeo.ie) Received: by mail-wr1-f51.google.com with SMTP id q5so3153958wrm.1 for ; Wed, 02 Jun 2021 10:40:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akeo-ie.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=6oefcDg6KCr46nmVGGCPIqgFLqzhSRjOkJrXtIVjcSE=; b=bXMZd/95rIw5pTAfRqsZtQD/jggOC9ZN/Fm50qcL9yQGLqCiNnIvllLBNVWxYbZl1V JfXdSCSzGSj63FWVBZDx+HxF6TkhP0Ffhlf/Q2KovmouvkOqUSg1hcYS4ahn9+YQrx7k QSr/ba4Ki+G31X407tcPhzyNaE+BZlxqOYGHhuGPcWLHnAI/ZXjZ22bMBRCQnKBERQYI yKIOoTYrK87IWbqdOmDHn0mLuzzbbSYAm7mS0T8FlAemvfQmMeDZrHE80eZ8S3Ye9Z0y YYRQ7qX4PaYQYpn6FOoXT0G1QHU5PvKiGoPveraC4zEAVJRFbQz9EHwz0LOuLWNEJoRm lMRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=6oefcDg6KCr46nmVGGCPIqgFLqzhSRjOkJrXtIVjcSE=; b=Vmjf8NDn7MS6kIuHvzjOuphh+XO51h0762To3c2wXAM11heeOyQBrlsidlxMzfRIBB 94HhxaX3nw1U3WWaOACESF5GG2Ua2wTrAIKIihk0eCmWUdPvURhnygc4MlKN+h3gZLkk p1YjTiwoStOxylTnYTW1ynUw/VFYa39Wn9Q/8Ox6N9r/fvrTj94yl7qu8oXr7bv6Xq7A 9gjhTx02fqijfQIoS6uyFFl1S/NNJmg7wYyLuizpLOyzod/BToB9jXc5X5tNm2Lwo/QI DU+MF7S+g3eEix2fGPCfxT46oEGCj66HXEj2NQ08EBcha1lRyOjZk+9VHMtLd4EFRHKW Zi9A== X-Gm-Message-State: AOAM532uuDMvum3WE6+V0eiN9VBVakhH9hIJO7J75w4W71ctWIuv9k0h KQB9lMdaTAqQiImNv8id+vUVOw== X-Google-Smtp-Source: ABdhPJylgYP767v/dMJbqw8Bw9Ibx0/YoYda2bplQGHH/JdCzA6vIIywQhPM/r0xPTi5geZWdRtIIg== X-Received: by 2002:a05:6000:1b8a:: with SMTP id r10mr34841691wru.296.1622655619206; Wed, 02 Jun 2021 10:40:19 -0700 (PDT) Return-Path: Received: from [10.0.0.122] ([84.203.86.196]) by smtp.googlemail.com with ESMTPSA id a12sm353492wmj.36.2021.06.02.10.40.17 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 02 Jun 2021 10:40:18 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v2 6/6] SecurityPkg: Add option to reset secure boot keys. To: devel@edk2.groups.io, gjb@semihalf.com Cc: leif@nuviainc.com, ardb+tianocore@kernel.org, Samer.El-Haj-Mahmoud@arm.com, sunny.Wang@arm.com, mw@semihalf.com, upstream@semihalf.com, jiewen.yao@intel.com, jian.j.wang@intel.com, min.m.xu@intel.com, lersek@redhat.com References: <20210601131229.630611-1-gjb@semihalf.com> <20210601131229.630611-8-gjb@semihalf.com> From: "Pete Batard" Message-ID: <592294cf-15e0-e24a-6c69-af2bcbbff28f@akeo.ie> Date: Wed, 2 Jun 2021 18:40:17 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.10.2 MIME-Version: 1.0 In-Reply-To: <20210601131229.630611-8-gjb@semihalf.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit On 2021.06.01 14:12, Grzegorz Bernacki wrote: > This commit add option which allows reset content of Secure Boot > keys and databases to default variables. > > Signed-off-by: Grzegorz Bernacki > --- > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf | 1 + > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h | 2 + > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr | 6 + > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 154 ++++++++++++++++++++ > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni | 4 + > 5 files changed, 167 insertions(+) > > diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > index 30d9cd8025..bd8d256dde 100644 > --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf > @@ -109,6 +109,7 @@ > [Protocols] > gEfiHiiConfigAccessProtocolGuid ## PRODUCES > gEfiDevicePathProtocolGuid ## PRODUCES > + gEfiHiiPopupProtocolGuid > > [Depex] > gEfiHiiConfigRoutingProtocolGuid AND > diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h > index 6e54a4b0f2..4ecc25efc3 100644 > --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigNvData.h > @@ -54,6 +54,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > #define KEY_VALUE_FROM_DBX_TO_LIST_FORM 0x100f > > +#define KEY_SECURE_BOOT_RESET_TO_DEFAULT 0x1010 > + > #define KEY_SECURE_BOOT_OPTION 0x1100 > #define KEY_SECURE_BOOT_PK_OPTION 0x1101 > #define KEY_SECURE_BOOT_KEK_OPTION 0x1102 > diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr > index fa7e11848c..e4560c592c 100644 > --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig.vfr > @@ -69,6 +69,12 @@ formset > endif; > endif; > > + text > + help = STRING_TOKEN(STR_SECURE_RESET_TO_DEFAULTS_HELP), > + text = STRING_TOKEN(STR_SECURE_RESET_TO_DEFAULTS), > + flags = INTERACTIVE, > + key = KEY_SECURE_BOOT_RESET_TO_DEFAULT; > + > endform; > > // > diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c > index 67e5e594ed..47f281873b 100644 > --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c > @@ -8,6 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > **/ > > #include "SecureBootConfigImpl.h" > +#include > #include > #include > > @@ -4154,6 +4155,132 @@ ON_EXIT: > return Status; > } > > +/** > + This function reinitializes Secure Boot variables with default values. > + > + @retval EFI_SUCCESS Success to update the signature list page > + @retval others Fail to delete or enroll signature data. > +**/ > + > +STATIC EFI_STATUS > +EFIAPI > +KeyEnrollReset ( > + VOID > + ) > +{ > + EFI_STATUS Status; > + UINT8 SetupMode; > + > + Status = EFI_SUCCESS; > + > + Status = SetSecureBootMode (CUSTOM_SECURE_BOOT_MODE); > + if (EFI_ERROR(Status)) { > + return Status; > + } > + > + // Clear all the keys and databases > + Status = DeleteDb (); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "Fail to clear DB: %r\n", Status)); > + return Status; > + } > + > + Status = DeleteDbx (); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "Fail to clear DBX: %r\n", Status)); > + return Status; > + } > + > + Status = DeleteDbt (); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "Fail to clear DBT: %r\n", Status)); > + return Status; > + } > + > + Status = DeleteKEK (); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "Fail to clear KEK: %r\n", Status)); > + return Status; > + } > + > + Status = DeletePlatformKey (); > + if (EFI_ERROR (Status) && (Status != EFI_NOT_FOUND)) { > + DEBUG ((DEBUG_ERROR, "Fail to clear PK: %r\n", Status)); > + return Status; > + } > + > + // After PK clear, Setup Mode shall be enabled > + Status = GetSetupMode (&SetupMode); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot get SetupMode variable: %r\n", > + Status)); > + return Status; > + } > + > + if (SetupMode == USER_MODE) { > + DEBUG((DEBUG_INFO, "Skipped - USER_MODE\n")); > + return EFI_SUCCESS; > + } > + > + Status = SetSecureBootMode (CUSTOM_SECURE_BOOT_MODE); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot set CUSTOM_SECURE_BOOT_MODE: %r\n", > + Status)); > + return EFI_SUCCESS; > + } > + > + // Enroll all the keys from default variables > + Status = EnrollDbFromDefault (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot enroll db: %r\n", Status)); > + goto error; > + } > + > + Status = EnrollDbxFromDefault (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot enroll dbx: %r\n", Status)); > + } > + > + Status = EnrollDbtFromDefault (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot enroll dbt: %r\n", Status)); > + } > + > + Status = EnrollKEKFromDefault (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot enroll KEK: %r\n", Status)); > + goto cleardbs; > + } > + > + Status = EnrollPKFromDefault (); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot enroll PK: %r\n", Status)); > + goto clearKEK; > + } > + > + Status = SetSecureBootMode (STANDARD_SECURE_BOOT_MODE); > + if (EFI_ERROR (Status)) { > + DEBUG ((DEBUG_ERROR, "Cannot set CustomMode to STANDARD_SECURE_BOOT_MODE\n" > + "Please do it manually, otherwise system can be easily compromised\n")); > + } > + > + return Status; > + > +clearKEK: > + DeleteKEK (); > + > +cleardbs: > + DeleteDbt (); > + DeleteDbx (); > + DeleteDb (); > + > +error: > + if (SetSecureBootMode (STANDARD_SECURE_BOOT_MODE) != EFI_SUCCESS) { > + DEBUG ((DEBUG_ERROR, "Cannot set mode to Secure: %r\n", Status)); > + } > + return Status; > +} > + > /** > This function is called to provide results data to the driver. > > @@ -4205,6 +4332,8 @@ SecureBootCallback ( > SECUREBOOT_CONFIG_PRIVATE_DATA *PrivateData; > BOOLEAN GetBrowserDataResult; > ENROLL_KEY_ERROR EnrollKeyErrorCode; > + EFI_HII_POPUP_PROTOCOL *HiiPopup; > + EFI_HII_POPUP_SELECTION UserSelection; > > Status = EFI_SUCCESS; > SecureBootEnable = NULL; > @@ -4755,6 +4884,31 @@ SecureBootCallback ( > FreePool (SetupMode); > } > break; > + case KEY_SECURE_BOOT_RESET_TO_DEFAULT: > + { > + Status = gBS->LocateProtocol (&gEfiHiiPopupProtocolGuid, NULL, (VOID **) &HiiPopup); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + Status = HiiPopup->CreatePopup ( > + HiiPopup, > + EfiHiiPopupStyleInfo, > + EfiHiiPopupTypeYesNo, > + Private->HiiHandle, > + STRING_TOKEN (STR_RESET_TO_DEFAULTS_POPUP), > + &UserSelection > + ); > + if (UserSelection == EfiHiiPopupSelectionYes) { > + Status = KeyEnrollReset (); > + } > + // > + // Update secure boot strings after key reset > + // > + if (Status == EFI_SUCCESS) { > + Status = UpdateSecureBootString (Private); > + SecureBootExtractConfigFromVariable (Private, IfrNvData); > + } > + } > default: > break; > } > diff --git a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni > index ac783453cc..0d01701de7 100644 > --- a/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni > +++ b/SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigStrings.uni > @@ -21,6 +21,10 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #string STR_SECURE_BOOT_PROMPT #language en-US "Attempt Secure Boot" > #string STR_SECURE_BOOT_HELP #language en-US "Enable/Disable the Secure Boot feature after platform reset" > > +#string STR_SECURE_RESET_TO_DEFAULTS_HELP #language en-US "Enroll keys with data from default variables" > +#string STR_SECURE_RESET_TO_DEFAULTS #language en-US "Reset Secure Boot Keys" > +#string STR_RESET_TO_DEFAULTS_POPUP #language en-US "Secure Boot Keys & databases will be initialized from defaults.\n Are you sure?" > + > #string STR_SECURE_BOOT_ENROLL_SIGNATURE #language en-US "Enroll Signature" > #string STR_SECURE_BOOT_DELETE_SIGNATURE #language en-US "Delete Signature" > #string STR_SECURE_BOOT_DELETE_LIST_FORM #language en-US "Delete Signature List Form" > Reviewed-by: Pete Batard Tested-by: Pete Batard on Raspberry Pi 4