From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f171.google.com (mail-pl1-f171.google.com [209.85.214.171]) by mx.groups.io with SMTP id smtpd.web08.11597.1605802515275985296 for ; Thu, 19 Nov 2020 08:15:15 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20161025 header.b=Kjyt5UlY; spf=pass (domain: gmail.com, ip: 209.85.214.171, mailfrom: debtech@gmail.com) Received: by mail-pl1-f171.google.com with SMTP id v21so1940291plo.12 for ; Thu, 19 Nov 2020 08:15:15 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=content-transfer-encoding:from:mime-version:subject:date:message-id :references:cc:in-reply-to:to; bh=43jNF2w2zra7haqygjlDszc1Ha4cGIW7BrYj/9QZiZs=; b=Kjyt5UlYc4MhFrTXtBnBl5H3g7GirdqX0vmDBVgvx72f3FZJMuT2PNnQs6YkJkGTwm bAm1g/u/vidh8U46nPMMNs6zM7bD2I/bmS2Pvq1uR+LOzLK/JT06XiBvef0ukYgT7tWh YdZc1z1/Fg2L42I1YWzjswgdYf+BEdDLDxSOkADqmbqg5lA7TKlh9Mso/uBRZDuq4C04 u/HdNo0dUKSbSEuDZ+TD3PnoJu3/qQHRfTaRlhnC3Fu04vVsWypq2ey41VOaXq6VzFGc NlXam2EAX2Qk9iMxfeGvteUGDST26oLMv9gF8cOcGf0bmk873bHDhpdZ0H5QB3/CCchA khxg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:content-transfer-encoding:from:mime-version :subject:date:message-id:references:cc:in-reply-to:to; bh=43jNF2w2zra7haqygjlDszc1Ha4cGIW7BrYj/9QZiZs=; b=BSo2iSBIt2itcZiZTaOUT5SJCUuU+9p/U4X97LrwxrLBx0TkjFBVPxwEiMCjqf++Bn XXDT7JaDWldE+2/paVlQ/uR02QkwanhVCiIoz3yPu41nzh3d6yIZgw4/MgI/jRxK1nlK EtfKJ0wGmosCCNjT7VkKlUaRTyB6Itaf5/vYE2+bb+aplPXTiSsvwC7qa7IhIpNfUx+i w0RQ3/HltsxEUheTv40d3N0uLpEfksb1r1qenA/MJoBDGEp3D1DXPsFAsnZlf2DnBmWd raYYAInYn6L7SyzSREfoYj5PkWwMznA6QB1JayP1WG3XaHyeffGd2U0XmiIWd58hZrMT rqqQ== X-Gm-Message-State: AOAM532tTWuKADCJZIzts5vD28projtob1TM4kO+to7rfG+TlV95G97s 6F+aTSvXjuvMzS9UuOGG7qM= X-Google-Smtp-Source: ABdhPJzkT76nBYZ27tIqbn5XvXH1Z3Gm1inQGt7T2X9WCTStC13XKpF6trjIKiT6ZKW2GlhLA4yNVA== X-Received: by 2002:a17:902:ea8c:b029:d6:e179:2097 with SMTP id x12-20020a170902ea8cb02900d6e1792097mr9917092plb.70.1605802514525; Thu, 19 Nov 2020 08:15:14 -0800 (PST) Return-Path: Received: from [192.168.50.139] ([71.212.128.184]) by smtp.gmail.com with ESMTPSA id a21sm38222pjq.37.2020.11.19.08.15.13 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 19 Nov 2020 08:15:13 -0800 (PST) From: Bret Barkelew Mime-Version: 1.0 (1.0) Subject: Re: [PATCH v9 00/13] Add the VariablePolicy feature Date: Thu, 19 Nov 2020 08:15:13 -0800 Message-Id: <345CC9C5-CCDB-4F76-A636-238B116DE217@gmail.com> References: <0c1ea26d-e7f2-11d7-b7d1-66cd5def51b3@arm.com> Cc: Bret Barkelew , devel@edk2.groups.io, Jiewen Yao , Dandan Bi , Chao Zhang , Jian J Wang , Hao A Wu , Liming Gao , Jordan Justen , Laszlo Ersek , Andrew Fish , Ray Ni , Bret Barkelew In-Reply-To: <0c1ea26d-e7f2-11d7-b7d1-66cd5def51b3@arm.com> To: Ard Biesheuvel X-Mailer: iPhone Mail (18A8395) Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Those bugs and recommendations were sent out months ago. Several platforms h= ave staged the changes already. You need to add the library class to your DSC. -- [ Insert obscure pop-culture reference here. ] > On Nov 19, 2020, at 4:46 AM, Ard Biesheuvel wrote= : >=20 > =EF=BB=BFOn 11/9/20 7:45 AM, 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 librari= es, >> a protocol, an SMI communication handler, and VariableServices integratio= n, >> the patches are broken up by individual library additions and then a fina= l >> 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-Prot= ocol---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 >=20 > This series has now made it into edk2, and has subsequently broken every s= ingle platform in edk2-platforms. Is anyone intending to propose any fixes f= or this? >=20 >=20 >> 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 D= einitVariablePolicyLib() >> * 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 platf= orms >> * 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 variab= les >> 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 gVariablePolicyProtocolG= uid >> 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 platform= s >> 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/VarCheckPolicy= Lib.c >> create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/Variable= PolicyHelperLib.c >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= ExtraInitNull.c >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= ExtraInitRuntimeDxe.c >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= Lib.c >> create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLo= ckRequestToLock.c >> create mode 100644 MdeModulePkg/Universal/Variable/RuntimeDxe/VariablePo= licySmmDxe.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/VarCheckPolicy= Lib.inf >> create mode 100644 MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicy= Lib.uni >> create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/Variable= PolicyHelperLib.inf >> create mode 100644 MdeModulePkg/Library/VariablePolicyHelperLib/Variable= PolicyHelperLib.uni >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/ReadMe.md >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= Lib.inf >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= Lib.uni >> create mode 100644 MdeModulePkg/Library/VariablePolicyLib/VariablePolicy= LibRuntimeDxe.inf >=20