Ard & Laszlo,
Variable store header is initialized by platform dsc, the merged variable
driver will AUTOMATICALLY compare EFI_VARIABLE_GUID/
EFI_AUTHENTICATED_VARIABLE_GUID against VARIABLE_STORE_HEADER.Signature to know
the format of variable (VARIABLE_HEADER or AUTHENTICATED_VARIABLE_HEADER(In
fact, it is VARIABLE_HEADER in AuthenticatedVariableFormat.h)). There is no
break to SECURE_BOOT_ENABLE = FALSE case. For example in VarStore.fdf.inc
!if $(SECURE_BOOT_ENABLE) == TRUE
# Signature: gEfiAuthenticatedVariableGuid =
# { 0xaaf32c78, 0x947b, 0x439a,
# { 0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92 }}
0x78, 0x2c, 0xf3, 0xaa, 0x7b, 0x94, 0x9a, 0x43,
0xa1, 0x80, 0x2e, 0x14, 0x4e, 0xc3, 0x77, 0x92,
!else
# Signature: gEfiVariableGuid =
# { 0xddcf3616, 0x3275, 0x4164,
# { 0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d }}
0x16, 0x36, 0xcf, 0xdd, 0x75, 0x32, 0x64, 0x41,
0x98, 0xb6, 0xfe, 0x85, 0x70, 0x7f, 0xfe, 0x7d,
!endif
AuthVariableLib will not to touch variable storage format(variable store header
or variable header), but only to provide auth services that follow UEFI spec.
I had been aware DxeTpmMeasurementLib in [LibraryClasses] section. I also found
BaseCryptLib|.../ RuntimeCryptLib.inf has been in
[LibraryClasses.common.DXE_RUNTIME_DRIVER] section and
OpensslLib|.../OpensslLib.inf has been in [LibraryClasses] section.
I just copied them to MdeModulePkg/.../VariableRuntimeDxe.inf from original
SecurityPkg/.../VariableRuntimeDxe.inf, and then add TpmMeasurementLib and
AuthVariableLib declaration.
SecurityPkg/VariableAuthenticated/RuntimeDxe/VariableRuntimeDxe.inf {
<LibraryClasses>
BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
}
Do you prefer the change to be like below in [LibraryClasses] section?
!if $(SECURE_BOOT_ENABLE) == TRUE
PlatformSecureLib|OvmfPkg/Library/PlatformSecureLib/PlatformSecureLib.inf
IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+ AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
!if $(NETWORK_IP6_ENABLE) == TRUE
TcpIoLib|MdeModulePkg/Library/DxeTcpIoLib/DxeTcpIoLib.inf
!endif
+!else
+
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+
AuthVariableLib|MdeModulePkg/Library/AuthVariableLibNull/AuthVariableLibNull.inf
!endif
Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
Sent: Thursday, June 25, 2015 7:45 PM
To: Laszlo Ersek
Cc: Zeng, Star; edk2-devel@lists.sourceforge.net; Justen, Jordan L
Subject: Re: [edk2] [PATCH 00/20] Separate auth variable service from Auth
Variable driver.
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/DxeTpmMeasu
> TpmMeasurementLib|rementLib.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
