From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.81]) by mx.groups.io with SMTP id smtpd.web10.2742.1591899213225723041 for ; Thu, 11 Jun 2020 11:13:33 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XkTVi8Rn; spf=pass (domain: redhat.com, ip: 207.211.31.81, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591899212; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=LqgJ++b++ih7BODBhIT9pTOoLeV7Q24xPQS74anQPq4=; b=XkTVi8RnDy2hGrmwAkl6wWjU/V+mYpK10kZQFz4AhLd45jHBLk2i58cfn+TrnyLr8eAWLI NaUwZgS5dnm3Vyv3AdRr9gBAm6L/BRx8adkjh5DMhXcR+PP4pvD33o6nzDuGJ3ZWoAqs50 rRsXzu65ow6xkIsq4Gk9w1gbALXi2Jo= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-237-tg7SjhNmPvSgfRvxG3c3eA-1; Thu, 11 Jun 2020 14:13:24 -0400 X-MC-Unique: tg7SjhNmPvSgfRvxG3c3eA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id DAB221883604; Thu, 11 Jun 2020 18:13:22 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-21.ams2.redhat.com [10.36.114.21]) by smtp.corp.redhat.com (Postfix) with ESMTP id D1E285D9DC; Thu, 11 Jun 2020 18:13:21 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v3 14/14] OvmfPkg: use generic QEMU image loader for secure boot enabled builds To: Ard Biesheuvel , 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> <56eff0cf-b76e-be82-db0e-b6a44a7f1908@arm.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 11 Jun 2020 20:13:20 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Firefox/52.0 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <56eff0cf-b76e-be82-db0e-b6a44a7f1908@arm.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit On 06/11/20 17:05, Ard Biesheuvel wrote: > 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. > Thanks. So... reverting this patch would only affect the behavior of the QemuLoadKernelImage() API, and that API only consumes fw_cfg. Fw_cfg is under the control of the host userspace (QEMU implements the fw_cfg "platform hardware). Does that satisfy your condition ("provided directly by the host userspace"), or are you referring to any "farther" origin on the host side, from where the fw_cfg content is originally taken? IOW would you involve in this decision the question where on the network the kernel image is downloaded from (on the host), for example? (I wouldn't -- for me, the fact that fw_cfg is technically controlled by QEMU is enough.) FWIW I've reverted the patch downstream (a deadline forced my hand before we could conclude this upstream thread, and the use cases that had regressed are considered important), but I really dislike that divergence from upstream. I'd like to eliminate that downstream patch when we rebase to a subsequent stable tag. Thanks, Laszlo