From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web12.14324.1605134073160471232 for ; Wed, 11 Nov 2020 14:34:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=ACUz+JTJ; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605134072; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Dvnol1Q9FKzdh+o9sBlovXagokq3cMxo1UHL5qwfkZM=; b=ACUz+JTJVyvNtuqUWl+pYTQ0QlFgInLXhoaXLSINeD4zpIjgW7o3xtm591+YPuiL2JWkTT fK4lXnaYYuPkObCPaUbPJE/1i02sXS9z5427nbPmvvBtVho+bw/BXSKo7e5Jf7lYwR+faR 3eOHxwOl8COMcIN5jyDjHrmJQ3L7IEE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-185-BFW_H02nNMGeGlfL-SFe2Q-1; Wed, 11 Nov 2020 17:34:29 -0500 X-MC-Unique: BFW_H02nNMGeGlfL-SFe2Q-1 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id ED721107B471; Wed, 11 Nov 2020 22:34:27 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-85.ams2.redhat.com [10.36.113.85]) by smtp.corp.redhat.com (Postfix) with ESMTP id 23EF755792; Wed, 11 Nov 2020 22:34:26 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v9 00/13] Add the VariablePolicy feature To: devel@edk2.groups.io, bret@corthon.com References: <20201109064522.919-1-bret.barkelew@microsoft.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 11 Nov 2020 23:34:26 +0100 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 11/11/20 19:43, Bret Barkelew wrote: > To clarify: > > The current solution to the MorLock EndOfDxe issue is to expressly call > LockVariablePolicy() in the same locations that mEndOfDxe is set (which was > the mechanism that previously locked the VariableLock interface). This > solution maintains parity with the old design, which is keeping with the > ethos of minimal changes and similar functionality to VariableLock. It does > not introduce any new dependencies. > > The only drawback to this approach is that it preserves the strict ordering > that was also previously required by MorLock, which I will attempt to > address in later updates. Thank you for the explanation! Laszlo > > On Sun, Nov 8, 2020 at 10:45 PM Bret Barkelew wrote: > >> The 14 patches in this series add the VariablePolicy feature to the core, >> deprecate Edk2VarLock (while adding a compatibility layer to reduce code >> churn), and integrate the VariablePolicy libraries and protocols into >> Variable Services. >> >> Since the integration requires multiple changes, including adding >> libraries, >> a protocol, an SMI communication handler, and VariableServices integration, >> the patches are broken up by individual library additions and then a final >> integration. Security-sensitive changes like bypassing Authenticated >> Variable enforcement are also broken out into individual patches so that >> attention can be called directly to them. >> >> Platform porting instructions are described in this wiki entry: >> >> https://github.com/tianocore/tianocore.github.io/wiki/VariablePolicy-Protocol---Enhanced-Method-for-Managing-Variables#platform-porting >> >> Discussion of the feature can be found in multiple places throughout >> the last year on the RFC channel, staging branches, and in devel. >> >> Most recently, this subject was discussed in this thread: >> https://edk2.groups.io/g/devel/message/53712 >> (the code branches shared in that discussion are now out of date, but the >> whitepapers and discussion are relevant). >> >> Cc: Jiewen Yao >> Cc: Dandan Bi >> Cc: Chao Zhang >> Cc: Jian J Wang >> Cc: Hao A Wu >> Cc: Liming Gao >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Cc: Andrew Fish >> Cc: Ray Ni >> Cc: Bret Barkelew >> Signed-off-by: Bret Barkelew >> >> v9 changes: >> * Rebase >> * Address the event ordering issues around MorLock at EndOfDxe >> * Drop problematic tests >> * Address ECC issues >> >> v8 changes: >> * Rebase >> * Small tweaks from final PRs >> * Drank a lot >> * Enrolled several members and a steward in CatFacts >> >> v7 changes: >> * Address comments from Dandan about security of the MM handler >> * Add readme >> * Fix bug around hex characters in BOOT####, etc >> * Add additional testing for hex characters >> * Add additional testing for authenticated variables >> >> v6 changes: >> * Fix an issue with uninitialized Status in InitVariablePolicyLib() and >> DeinitVariablePolicyLib() >> * Fix GCC building in shell-based functional test >> * Rebase on latest origin/master >> >> v5 changes: >> * Fix the CONST mismatch in VariablePolicy.h and VariablePolicySmmDxe.c >> * Fix EFIAPI mismatches in the functional unittest >> * Rebase on latest origin/master >> >> v4 changes: >> * Remove Optional PcdAllowVariablePolicyEnforcementDisable PCD from >> platforms >> * Rebase on master >> * Migrate to new MmCommunicate2 protocol >> * Fix an oversight in the default return value for InitMmCommonCommBuffer >> * Fix in VariablePolicyLib to allow ExtraInitRuntimeDxe to consume >> variables >> >> V3 changes: >> * Address all non-unittest issues with ECC >> * Make additional style changes >> * Include section name in hunk headers in "ini-style" files >> * Remove requirement for the EdkiiPiSmmCommunicationsRegionTable driver >> (now allocates its own buffer) >> * Change names from VARIABLE_POLICY_PROTOCOL and >> gVariablePolicyProtocolGuid >> to EDKII_VARIABLE_POLICY_PROTOCOL and gEdkiiVariablePolicyProtocolGuid >> * Fix GCC warning about initializing externs >> * Add UNI strings for new PCD >> * Add patches for ArmVirtPkg, OvmfXen, and UefiPayloadPkg >> * Reorder patches according to Liming's feedback about adding to platforms >> before changing variable driver >> >> V2 changes: >> * Fixed implementation for RuntimeDxe >> * Add PCD to block DisableVariablePolicy >> * Fix the DumpVariablePolicy pagination in SMM >> >> Bret Barkelew (13): >> MdeModulePkg: Define the VariablePolicy protocol interface >> MdeModulePkg: Define the VariablePolicyLib >> MdeModulePkg: Define the VariablePolicyHelperLib >> MdeModulePkg: Define the VarCheckPolicyLib and SMM interface >> OvmfPkg: Add VariablePolicy engine to OvmfPkg platform >> EmulatorPkg: Add VariablePolicy engine to EmulatorPkg platform >> ArmVirtPkg: Add VariablePolicy engine to ArmVirtPkg platform >> UefiPayloadPkg: Add VariablePolicy engine to UefiPayloadPkg platform >> MdeModulePkg: Connect VariablePolicy business logic to >> VariableServices >> MdeModulePkg: Allow VariablePolicy state to delete protected variables >> SecurityPkg: Allow VariablePolicy state to delete authenticated >> variables >> MdeModulePkg: Change TCG MOR variables to use VariablePolicy >> MdeModulePkg: Drop VarLock from RuntimeDxe variable driver >> >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c >> | 346 ++++++++ >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c >> | 396 ++++++++++ >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c >> | 46 ++ >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c >> | 85 ++ >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c >> | 830 ++++++++++++++++++++ >> MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockDxe.c >> | 52 +- >> MdeModulePkg/Universal/Variable/RuntimeDxe/TcgMorLockSmm.c >> | 60 +- >> MdeModulePkg/Universal/Variable/RuntimeDxe/VarCheck.c >> | 49 +- >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableDxe.c >> | 60 ++ >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c >> | 71 ++ >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c >> | 573 ++++++++++++++ >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c >> | 7 + >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c >> | 14 + >> SecurityPkg/Library/AuthVariableLib/AuthService.c >> | 30 +- >> ArmVirtPkg/ArmVirt.dsc.inc >> | 4 + >> EmulatorPkg/EmulatorPkg.dsc >> | 3 + >> MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h >> | 54 ++ >> MdeModulePkg/Include/Library/VariablePolicyHelperLib.h >> | 164 ++++ >> MdeModulePkg/Include/Library/VariablePolicyLib.h >> | 207 +++++ >> MdeModulePkg/Include/Protocol/VariablePolicy.h >> | 157 ++++ >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf >> | 42 + >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni >> | 12 + >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf >> | 35 + >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni >> | 12 + >> MdeModulePkg/Library/VariablePolicyLib/ReadMe.md >> | 406 ++++++++++ >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf >> | 48 ++ >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni >> | 12 + >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf >> | 51 ++ >> MdeModulePkg/MdeModulePkg.ci.yaml >> | 4 +- >> MdeModulePkg/MdeModulePkg.dec >> | 26 +- >> MdeModulePkg/MdeModulePkg.dsc >> | 9 + >> MdeModulePkg/MdeModulePkg.uni >> | 7 + >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf >> | 5 + >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf >> | 4 + >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf >> | 11 + >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf >> | 4 + >> OvmfPkg/OvmfPkgIa32.dsc >> | 5 + >> OvmfPkg/OvmfPkgIa32X64.dsc >> | 5 + >> OvmfPkg/OvmfPkgX64.dsc >> | 5 + >> OvmfPkg/OvmfXen.dsc >> | 4 + >> SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf >> | 2 + >> UefiPayloadPkg/UefiPayloadPkgIa32.dsc >> | 4 + >> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc >> | 4 + >> 43 files changed, 3845 insertions(+), 80 deletions(-) >> create mode 100644 >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.c >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitNull.c >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyExtraInitRuntimeDxe.c >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c >> create mode 100644 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c >> create mode 100644 >> MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePolicySmmDxe.c >> create mode 100644 MdeModulePkg/Include/Guid/VarCheckPolicyMmi.h >> create mode 100644 MdeModulePkg/Include/Library/VariablePolicyHelperLib.h >> create mode 100644 MdeModulePkg/Include/Library/VariablePolicyLib.h >> create mode 100644 MdeModulePkg/Include/Protocol/VariablePolicy.h >> create mode 100644 >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf >> create mode 100644 >> MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.uni >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.uni >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/ReadMe.md >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.uni >> create mode 100644 >> MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf >> >> -- >> 2.28.0.windows.1 >> >> > > > > > >