On 25 June 2015 at 13:15, Laszlo Ersek <ler...@redhat.com> wrote: > On 06/25/15 11:46, Star Zeng wrote: >> For your easy review, >> the forked code is at g...@github.com:lzeng14/edk2.git branch >> SeparateAuthVariableService. >> >> What to do: >> 1. Move TpmMeasurementLib LibraryClass from SecurityPkg to MdeModulePkg. >> 2. Implement a NULL TpmMeasurementLib library instance in MdeModulePkg. >> 3. Move authenticated variable definition from AuthenticatedVariableFormat.h >> to VariableFormat.h. >> 4. Merge VariableInfo in SecurityPkg to VariableInfo in MdeModulePkg. >> 5. Merge from VariablePei in SecurityPkg to VariablePei in MdeModulePkg. >> 6. Add AuthVariableLib LibraryClass definitions in MdeModulePkg. >> 7. Implement a NULL AuthVariableLib library instance in MdeModulePkg. >> 8. Implement AuthVariableLib library instance in SecurityPkg. >> 9. Merge from Auth Variable driver in SecurityPkg to Variable drive in >> MdeModulePkg. >> 10. Update platform package to use the merged Variable driver. >> >> Why to do: >> 1. Share code. >> We are moving forward to separate auth variable service from Auth Variable >> driver in SecurityPkg to AuthVariableLib. Then the AuthVariableLib could >> benefit >> and be used by different implementation of Auth Variable drivers. >> 2. Remove code duplication and reduce maintenance effort. >> 2.1. After auth variable service separated from Auth Variable driver in >> SecurityPkg >> to AuthVariableLib. The remaining code logic of Auth Variable driver in >> SecurityPkg >> will be almost same with Variable driver in MdeModulePkg. We are going to >> merge them. >> 2.2. The functionality of VariableInfo in SecurityPkg has covered >> VariableInfo >> in MdeModulePkg. >> 2.3. The code logic of VariablePei in SecurityPkg is same with VariablePei >> in MdeModulePkg. >> 3. TpmMeasurementLib is consumed by Auth Variable driver in SecurityPkg now, >> as Auth Variable driver in SecurityPkg will be merged to Variable driver in >> MdeModulePkg, so the library class also needs to be moved to MdeModulePkg. >> 4. gEfiAuthenticatedVariableGuid will be used by both merged Variable driver >> and AuthVariableLib, AUTHENTICATED_VARIABLE_HEADER will be used by >> merged Variable driver. >> >> What test done: >> Nt32: Boot with SECURE_BOOT_ENABLE = TRUE or FALSE, enable secure boot with >> SECURE_BOOT_ENABLE = TRUE. >> OVMF: Boot with SECURE_BOOT_ENABLE = TRUE or FALSE, enable secure boot with >> SECURE_BOOT_ENABLE = TRUE. >> Vlv2TbltDevice: Boot and enable secure boot with SECURE_BOOT_ENABLE = TRUE. >> >> What is the impact to platform: >> 1. Only platform dsc and fdf need to be updated except the change in >> ArmPlatformPkg.dec and NorFlashAuthenticatedDxe.inf to remove >> gVariableAuthenticatedRuntimeDxeFileGuid and use gVariableRuntimeDxeFileGuid. > > Since Ard added secure boot support to ArmVirtPkg, I will let him review > patch 19/20. > > With regard to OVMF, I have a question, and some notes. > > * Question: In point 4 above, do I understand correctly that the > varstore is now supposed to have only one GUID, the authenticated one, > regardless of whether secure boot is enabled in the build or not? > > If that's the case, then it's problematic. > > First, technically, "OvmfPkg/VarStore.fdf.inc" will be broken for the > SECURE_BOOT_ENABLE=FALSE case, because this FDF include file will build > a varstore template that will be then rejected by the variable driver. > > Second, such an update will break all preexistent virtual machines (and, > well, physical machines too) that use firmware with > SECURE_BOOT_ENABLE=FALSE. Their current varstores contain the > non-authenticated GUID, and after a firmware upgrade (despite *keeping* > SECURE_BOOT_ENABLE=FALSE), those varstores will be rejected. (If I > understand right.) > > I think that the new, merged, variable driver should be parametrized, > with regard to the varstore GUID. The driver should take a fixed pointer > PCD, to a 16 byte array, specifying the GUID, and DSC files should be > able to specify that PCD, based on SECURE_BOOT_ENABLE. >
This should probably be moved to AuthVariableLib, i.e., it should have a function that returns the appropriate GUID for non-authenticated resp authenticated variable stores. > * Notes (for patch 16/20): > > Please grep OvmfPkg for all instances of SECURE_BOOT_ENABLE, and adapt > them all as appropriate. > > The patch currently adds one new conditional and removes one in each DSC > file, and modifies one conditional in each FDF file. > > That's not good enough. Beyond the VarStore.fdf.inc issue mentioned > above, each DSC file contains 8 "SECURE_BOOT_ENABLE" conditionals, and > *some* of those overlap with your patch. For example, the > > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf > > library resolution is already present elsewhere. > > So please investigate all current SECURE_BOOT_ENABLE conditionals under > OvmfPkg/, and update them / simplify them all as appropriate. > > Thanks > Laszlo ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
