From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web12.9482.1591887926737432352 for ; Thu, 11 Jun 2020 08:05:26 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5C4561F1; Thu, 11 Jun 2020 08:05:26 -0700 (PDT) Received: from [192.168.1.69] (unknown [10.37.8.184]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 602463F6CF; Thu, 11 Jun 2020 08:05:25 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v3 14/14] OvmfPkg: use generic QEMU image loader for secure boot enabled builds To: Laszlo Ersek , devel@edk2.groups.io Cc: =?UTF-8?Q?Philippe_Mathieu-Daud=c3=a9?= References: <20200305134607.20125-1-ard.biesheuvel@linaro.org> <20200305134607.20125-15-ard.biesheuvel@linaro.org> <5243a882-187f-6ca4-2450-a6c958a934e3@redhat.com> <7d7d1cbc-d1ca-c3dd-c2ac-00084d79decf@redhat.com> <20eb271e-ac27-c097-e933-2d2273dfb4d5@arm.com> <85097379-850e-ece2-13e7-eefeb30d12ed@redhat.com> From: "Ard Biesheuvel" Message-ID: <56eff0cf-b76e-be82-db0e-b6a44a7f1908@arm.com> Date: Thu, 11 Jun 2020 17:05:22 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: <85097379-850e-ece2-13e7-eefeb30d12ed@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 6/11/20 4:55 PM, Laszlo Ersek wrote: > Hi Ard, >=20 > On 06/10/20 11:32, Ard Biesheuvel wrote: >> On 6/10/20 11:22 AM, Laszlo Ersek wrote: >=20 >>> (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 th= e >>> X86QemuLoadImageLib instance, we'd just predicate the fallback to the >>> legacy Linux/x86 boot protocol on: >>> >>> =C2=A0=C2=A0 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 enoug= h" >>> 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 save= d >>> 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 anot= her >>>> 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 a= re >>>> loading a signed image from a known source, and this whole dance wit= h >>>> shim and the MS certs is ridiculous, and I am still hoping we can av= oid >>>> 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 mor= e >>> 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 entri= es: >>> >>> - 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 f= or >>> 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 rig= ht >> thing to do regardless - going around installing the MS certs everywhe= re >> is a recipe for disaster, and AIUI, MS now requires you to remove them >> in some cases?? (for DeviceGuard support or sth like that) >=20 > 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: >=20 > - the problem will affect every guest distro different from SUSE and RH > out there, and every host distro providing KVM hypervisor, >=20 > - the change isn't fixing a practical security issue. >=20 > (I'm quite stressed about bringing this internal feedback to you, > admittedly...) >=20 Don't worry about it. If we can ensure that the only bootable images that are exempt from the=20 secure boot checks are ones that were provided directly by the host=20 userspace, then I think their position is not unreasonable, given that=20 the guest is at its mercy anyway.