From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.9274.1591887312928533473 for ; Thu, 11 Jun 2020 07:55:13 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=XcSe9hjN; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1591887312; 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=DobzugfDvuNo/Yq94dIyYyzjHJwpNKBoDn8lPhLJN6E=; b=XcSe9hjNEpeJ46Fc9B/JInB+ldoEKPPNQIa5YMghH4bOcvbpx4S0F0E6XSGL4EyAewnycG D3K/XbT4g1sx9l5DeKHWXbu1g0j9anCdY6TX4NJNqjLHhjzeFP4WkVh42e7SdF6AdYePSD hZXGF0awIBMoNFvGYEBdf5U9wrFv8sk= 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-254-sUXvcaSSODmI4Go16TC6TA-1; Thu, 11 Jun 2020 10:55:10 -0400 X-MC-Unique: sUXvcaSSODmI4Go16TC6TA-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 43521835B40; Thu, 11 Jun 2020 14:55:09 +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 31CE95D9DC; Thu, 11 Jun 2020 14:55:08 +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> From: "Laszlo Ersek" Message-ID: <85097379-850e-ece2-13e7-eefeb30d12ed@redhat.com> Date: Thu, 11 Jun 2020 16:55:07 +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: <20eb271e-ac27-c097-e933-2d2273dfb4d5@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=WINDOWS-1252 Content-Transfer-Encoding: 8bit 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...) Thanks, Laszlo