Re: [edk2-devel] [PATCH v3 14/14] OvmfPkg: use generic QEMU image loader for secure boot enabled builds

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

On 6/11/20 4:55 PM, Laszlo Ersek wrote:
> Hi Ard,
> 
> On 06/10/20 11:32, Ard Biesheuvel wrote:
>> On 6/10/20 11:22 AM, Laszlo Ersek wrote:
> 
>>> (3) make it a dynamic boolean PCD, and use the recently introduced
>>> fw_cfg control interface
>>>
>>>
>>> In all three options, the OVMF DSC files would unconditionally use the
>>> X86QemuLoadImageLib instance, we'd just predicate the fallback to the
>>> legacy Linux/x86 boot protocol on:
>>>
>>>    SecureBoot disabled || FallbackPcd
>>>
>>> (where "SecureBoot disabled" means that the SecureBoot standard UEFI
>>> variable doesn't exist, or has zero value).
>>>
>>> I think option (2) means the least churn, and it would be "good enough"
>>> functionally.
>>>
>>> Does this look palatable to you?
>>>
>>>
>>>> So what is the point of DEFER_EXECUTE_ON_SECURITY_VIOLATION anyway, if
>>>> you cannot start the image anyway?
>>>
>>> Honestly: I'm not sure!
>>>
>>>> Because I would prefer it if we could
>>>> start the image anyway, Unfortunately, the only way seems to be
>>>
>>> Right; it seems like the EFI_SECURITY_VIOLATION status would get saved
>>> in the private LoadImageStatus field, and that would cause StartImage()
>>> to fail early as well. I couldn't find a spot to "re-set"
>>> LoadImageStatus.
>>>
>>>> to use
>>>> the loaded image protocol data (base and size in memory) and do another
>>>> LoadImage() after adding the hash to "db", and this is also more
>>>> elaborate than I would like.
>>>
>>> Agreed. We shouldn't mess with "db".
>>>
>>>> Of course, the *correct* fix is to add the vendor cert to db - you are
>>>> loading a signed image from a known source, and this whole dance with
>>>> shim and the MS certs is ridiculous, and I am still hoping we can avoid
>>>> this mess on ARM
>>>
>>> I considered this. It didn't look feasible to me at first sight (how
>>> many vendors?), but now that you mention it and I've looked at it more
>>> closely, it does look feasible. Selfishly (!), if I look at the guest OS
>>> types officially supported on RHEL8 hosts:
>>>
>>> - https://access.redhat.com/articles/973133
>>> - https://access.redhat.com/articles/973163
>>>
>>> then all we (RH) would need are two patches for (upstream)
>>> OvmfPkg/EnrollDefaultKeys. Namely, we should enroll two more DB entries:
>>>
>>> - SUSE's CA certificate to which the key(s) chain that they use for
>>> signing their kernels
>>>
>>> - Red Hat's CA certificate to which the key(s) chain that we use for
>>> signing our kernels.
>>>
>>> That would strictly be an improvement even for upstream OVMF (more
>>> vendors accepted).
>>>
>>> Ultimately I think the right way is to pick option (3) -- dynamic PCD --
>>> *and* add the SUSE and RH CA certs to OvmfPkg/EnrollDefaultKeys. That
>>> means SUSE and RH signed kernels can be booted at once, with secure
>>> verification, and users would still have a QEMU command line option for
>>> opting *into* the legacy boot protocol fallback.
>>>
>>> Do you agree? Would you be OK with patches to this effect?
>>>
>>
>> Yes, option #3 looks reasonable to me. And adding the certs is the right
>> thing to do regardless - going around installing the MS certs everywhere
>> is a recipe for disaster, and AIUI, MS now requires you to remove them
>> in some cases?? (for DeviceGuard support or sth like that)
> 
> I've "escalated" this internally to other virt team engineers, and they
> seem to consider this a plain regression. They suggested we should
> revert the patch, because:
> 
> - the problem will affect every guest distro different from SUSE and RH
> out there, and every host distro providing KVM hypervisor,
> 
> - the change isn't fixing a practical security issue.
> 
> (I'm quite stressed about bringing this internal feedback to you,
> admittedly...)
> 

Don't worry about it.

If we can ensure that the only bootable images that are exempt from the 
secure boot checks are ones that were provided directly by the host 
userspace, then I think their position is not unreasonable, given that 
the guest is at its mercy anyway.