From nobody Mon Sep 16 19:43:22 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) client-ip=170.10.133.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1679081297; cv=none; d=zohomail.com; s=zohoarc; b=OXDcJtM+m0Q0HE9MSw0OOymFuzdFCZgSJA+rT7F6BOsFWUX64TSGJ22hLy2/LSBvLvbERsNAq3yo+MSs2gPDGiRLo21gt0IDydWbDHEJrpwD7BDhETO5MxOQhi6XMhMHQRnxhoaTXKTdngPgAVH4ZGk8EvA7A3ys2DKoUEOw2Ss= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1679081297; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=kX1UKeW6xklG+ZsOTgiyvDT5vyAh1XHRWE7kwrDoMrc=; b=lzAgueRjBD4uJWJqZbiTIlcBQKkfWuW94+An0+knGk1TNFjYHOqjXXIivVzAsu0vzCDyqQrUhAcGNJR8n7fNFG7BRk5um0S40Xt+gayqMTrNBcgvIuHrTj5ODrBYkfmBr6J7u/8Lp0ehaOvNejxKli4q1d1RjGR0piJUYKGnhKU= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.zohomail.com with SMTPS id 167908129770881.34392480765678; Fri, 17 Mar 2023 12:28:17 -0700 (PDT) Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-320-mSIgb6gOPj2RaIF757LfMw-1; Fri, 17 Mar 2023 15:28:12 -0400 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2CADE1C189A3; Fri, 17 Mar 2023 19:28:04 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (unknown [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 16F302166B29; Fri, 17 Mar 2023 19:28:04 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 0B7531946A4A; Fri, 17 Mar 2023 19:28:04 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.rdu2.redhat.com [10.11.54.2]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 2E30D1946A4B for ; Fri, 17 Mar 2023 19:27:56 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id 1D36840C6E67; Fri, 17 Mar 2023 19:27:56 +0000 (UTC) Received: from harajuku.usersys.redhat.com.homenet.telecomitalia.it (unknown [10.45.226.11]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 834BC40C6F87 for ; Fri, 17 Mar 2023 19:27:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1679081296; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=kX1UKeW6xklG+ZsOTgiyvDT5vyAh1XHRWE7kwrDoMrc=; b=iQipOFdM7e8QrveNhtB3JwC8AnVjDN3Ady4cTCDH+5dAuPzFXT1K/AEJvISktJOtfwMZF8 v3x3oqC5RJ4VccyVdh5dZLtR0QG6xBQy0HyK9huUvnBHPIA5rfEUmrTEpHocNvCrMoeab6 zn2ifV/vmC5Bz3uv6SVfoxXOPwC+YtE= X-MC-Unique: mSIgb6gOPj2RaIF757LfMw-1 X-Original-To: libvir-list@listman.corp.redhat.com From: Andrea Bolognani To: libvir-list@redhat.com Subject: [libvirt PATCH 09/15] conf: Remove some firmware validation checks Date: Fri, 17 Mar 2023 20:27:41 +0100 Message-Id: <20230317192747.1311223-10-abologna@redhat.com> In-Reply-To: <20230317192747.1311223-1-abologna@redhat.com> References: <20230317192747.1311223-1-abologna@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.2 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1679081298758100003 Content-Type: text/plain; charset="utf-8"; x-default="true" libvirt 8.6.0 introduced these checks and very clearly delineated two possible firmware selection scenarios: manual firmware selection, where the user is responsible for providing all information, and firmware autoselection, where a list of desired features is provided and everything else is handled by libvirt. In the interest of maintaining the clear separation between these two scenarios, setting most attributes when firmware autoselection is active will result in the configuration being rejected. This works fine, but is unnecessarily restrictive: in most cases, the additional information that the user has provided matches the information that libvirt would have discovered on its own by looking at firmware descriptors, and asking the user to scrub it from the XML only result in pointless friction. Remove these checks entirely. Unsurprisingly, this results in a few test cases that were rejected until now to suddenly start working and producing sensible results. The firmware-auto-efi-loader-path-nonstandard test case is notable: while we can now enable the xml2xml part of the test, the xml2argv part is still failing, although in a slightly different way. This is expected: since the firmware binary is a non-standard one, libvirt is unable to figure out the missing information from a firmware descriptor, and the configuration is still ultimately an invalid one. However, if we were to find such a configuration on disk at daemon startup, we would not ignore it completely and instead would offer the user a chance to fix it. Signed-off-by: Andrea Bolognani --- src/conf/domain_validate.c | 38 ------------------- ...uto-efi-loader-insecure.x86_64-latest.args | 37 ++++++++++++++++++ ...auto-efi-loader-insecure.x86_64-latest.err | 1 - ...-loader-path-nonstandard.x86_64-latest.err | 2 +- ...re-auto-efi-loader-path.x86_64-latest.args | 37 ++++++++++++++++++ ...are-auto-efi-loader-path.x86_64-latest.err | 1 - tests/qemuxml2argvtest.c | 6 +-- ...auto-efi-loader-insecure.x86_64-latest.xml | 36 ++++++++++++++++++ ...-loader-path-nonstandard.x86_64-latest.xml | 35 +++++++++++++++++ ...are-auto-efi-loader-path.x86_64-latest.xml | 36 ++++++++++++++++++ tests/qemuxml2xmltest.c | 3 ++ 11 files changed, 188 insertions(+), 44 deletions(-) create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecur= e.x86_64-latest.args delete mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-insecur= e.x86_64-latest.err create mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x8= 6_64-latest.args delete mode 100644 tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x8= 6_64-latest.err create mode 100644 tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insec= ure.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path-= nonstandard.x86_64-latest.xml create mode 100644 tests/qemuxml2xmloutdata/firmware-auto-efi-loader-path.= x86_64-latest.xml diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 5fb2d4971c..6991cf1dd3 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1609,44 +1609,6 @@ virDomainDefOSValidate(const virDomainDef *def, if (!loader) return 0; =20 - if (loader->readonly) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("loader attribute 'readonly' cannot be specif= ied " - "when firmware autoselection is enabled")); - return -1; - } - if (loader->type) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("loader attribute 'type' cannot be specified " - "when firmware autoselection is enabled")); - return -1; - } - if (loader->path) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("loader path cannot be specified " - "when firmware autoselection is enabled")); - return -1; - } - if (loader->nvramTemplate) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("nvram attribute 'template' cannot be specifi= ed " - "when firmware autoselection is enabled")); - return -1; - } - - /* We need to accept 'yes' here because the initial implementation - * of firmware autoselection used it as a way to request a firmware - * with Secure Boot support, so the error message is technically - * incorrect; however, we want to discourage people from using this - * attribute at all, so it's fine to be a bit more aggressive than - * it would be strictly required :) */ - if (loader->secure =3D=3D VIR_TRISTATE_BOOL_NO) { - virReportError(VIR_ERR_XML_DETAIL, "%s", - _("loader attribute 'secure' cannot be specifie= d " - "when firmware autoselection is enabled")); - return -1; - } - if (loader->nvram && def->os.firmware !=3D VIR_DOMAIN_OS_DEF_FIRMW= ARE_EFI) { virReportError(VIR_ERR_XML_DETAIL, _("firmware type '%s' does not support nvram"), diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_6= 4-latest.args b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x8= 6_64-latest.args new file mode 100644 index 0000000000..9326bfe305 --- /dev/null +++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-lates= t.args @@ -0,0 +1,37 @@ +LC_ALL=3DC \ +PATH=3D/bin \ +HOME=3D/var/lib/libvirt/qemu/domain--1-guest \ +USER=3Dtest \ +LOGNAME=3Dtest \ +XDG_DATA_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=3Dguest,debug-threads=3Don \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/va= r/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","nod= e-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'= \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver"= :"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_= VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"disca= rd":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver= ":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc-q35-4.0,usb=3Doff,dump-guest-core=3Doff,memory-backend=3Dpc.ra= m,pflash0=3Dlibvirt-pflash0-format,pflash1=3Dlibvirt-pflash1-format,acpi=3D= on \ +-accel kvm \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}= ' \ +-overcommit mem-lock=3Doff \ +-smp 1,sockets=3D1,cores=3D1,threads=3D1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=3Dcharmonitor,fd=3D1729,server=3Don,wait=3Doff \ +-mon chardev=3Dcharmonitor,id=3Dmonitor,mode=3Dcontrol \ +-rtc base=3Dutc \ +-no-shutdown \ +-boot strict=3Don \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-global ICH9-LPC.noreboot=3Doff \ +-watchdog-action reset \ +-sandbox on,obsolete=3Ddeny,elevateprivileges=3Ddeny,spawn=3Ddeny,resource= control=3Ddeny \ +-msg timestamp=3Don diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_6= 4-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86= _64-latest.err deleted file mode 100644 index 564f0e6918..0000000000 --- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-insecure.x86_64-lates= t.err +++ /dev/null @@ -1 +0,0 @@ -loader attribute 'secure' cannot be specified when firmware autoselection = is enabled diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstanda= rd.x86_64-latest.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path= -nonstandard.x86_64-latest.err index 3f90a88791..4cfde1bd2e 100644 --- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_= 64-latest.err +++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path-nonstandard.x86_= 64-latest.err @@ -1 +1 @@ -loader attribute 'readonly' cannot be specified when firmware autoselectio= n is enabled +operation failed: Unable to find any firmware to satisfy 'efi' diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-la= test.args b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-lat= est.args new file mode 100644 index 0000000000..9326bfe305 --- /dev/null +++ b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.ar= gs @@ -0,0 +1,37 @@ +LC_ALL=3DC \ +PATH=3D/bin \ +HOME=3D/var/lib/libvirt/qemu/domain--1-guest \ +USER=3Dtest \ +LOGNAME=3Dtest \ +XDG_DATA_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=3D/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-x86_64 \ +-name guest=3Dguest,debug-threads=3Don \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/va= r/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-blockdev '{"driver":"file","filename":"/usr/share/OVMF/OVMF_CODE.fd","nod= e-name":"libvirt-pflash0-storage","auto-read-only":true,"discard":"unmap"}'= \ +-blockdev '{"node-name":"libvirt-pflash0-format","read-only":true,"driver"= :"raw","file":"libvirt-pflash0-storage"}' \ +-blockdev '{"driver":"file","filename":"/var/lib/libvirt/qemu/nvram/guest_= VARS.fd","node-name":"libvirt-pflash1-storage","auto-read-only":true,"disca= rd":"unmap"}' \ +-blockdev '{"node-name":"libvirt-pflash1-format","read-only":false,"driver= ":"raw","file":"libvirt-pflash1-storage"}' \ +-machine pc-q35-4.0,usb=3Doff,dump-guest-core=3Doff,memory-backend=3Dpc.ra= m,pflash0=3Dlibvirt-pflash0-format,pflash1=3Dlibvirt-pflash1-format,acpi=3D= on \ +-accel kvm \ +-cpu qemu64 \ +-m 1024 \ +-object '{"qom-type":"memory-backend-ram","id":"pc.ram","size":1073741824}= ' \ +-overcommit mem-lock=3Doff \ +-smp 1,sockets=3D1,cores=3D1,threads=3D1 \ +-uuid 63840878-0deb-4095-97e6-fc444d9bc9fa \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=3Dcharmonitor,fd=3D1729,server=3Don,wait=3Doff \ +-mon chardev=3Dcharmonitor,id=3Dmonitor,mode=3Dcontrol \ +-rtc base=3Dutc \ +-no-shutdown \ +-boot strict=3Don \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-global ICH9-LPC.noreboot=3Doff \ +-watchdog-action reset \ +-sandbox on,obsolete=3Ddeny,elevateprivileges=3Ddeny,spawn=3Ddeny,resource= control=3Ddeny \ +-msg timestamp=3Don diff --git a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-la= test.err b/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-late= st.err deleted file mode 100644 index 3f90a88791..0000000000 --- a/tests/qemuxml2argvdata/firmware-auto-efi-loader-path.x86_64-latest.err +++ /dev/null @@ -1 +0,0 @@ -loader attribute 'readonly' cannot be specified when firmware autoselectio= n is enabled diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 23e48b251c..bd3f46fbe0 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -1118,9 +1118,9 @@ mymain(void) DO_TEST_CAPS_LATEST("firmware-auto-efi-stateless"); DO_TEST_CAPS_LATEST("firmware-auto-efi-nvram"); DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-secure"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-insecure"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-path"); - DO_TEST_CAPS_LATEST_PARSE_ERROR("firmware-auto-efi-loader-path-nonstan= dard"); + DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-insecure"); + DO_TEST_CAPS_LATEST("firmware-auto-efi-loader-path"); + DO_TEST_CAPS_LATEST_FAILURE("firmware-auto-efi-loader-path-nonstandard= "); DO_TEST_CAPS_LATEST("firmware-auto-efi-secboot"); DO_TEST_CAPS_LATEST("firmware-auto-efi-no-secboot"); DO_TEST_CAPS_LATEST("firmware-auto-efi-enrolled-keys"); diff --git a/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure.x86= _64-latest.xml b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure= .x86_64-latest.xml new file mode 100644 index 0000000000..a6af5512d3 --- /dev/null +++ b/tests/qemuxml2xmloutdata/firmware-auto-efi-loader-insecure.x86_64-lat= est.xml @@ -0,0 +1,36 @@ + + guest + 63840878-0deb-4095-97e6-fc444d9bc9fa + 1048576 + 1048576 + 1 + + hvm + /usr/share/OVMF= /OVMF_CODE.fd + /var/lib/libvirt/qemu= /nvram/guest_VARS.fd + + + + + + + qemu64 + + + destroy + restart + destroy + + /usr/bin/qemu-system-x86_64 + + +
+ + + + +