[libvirt] [PATCH v5 00/20] Add support for vTPM state encryption
This series of patches addresses the RFE in BZ 172830: https://bugzilla.redhat.com/show_bug.cgi?id=1728030 This series of patches adds support for vTPM state encryption by passing the read-end of a pipe's file descriptor to 'swtpm_setup' and 'swtpm' where they can read a passphrase from and derive a key from that passphrase. The TPM's domain XML looks to enable state encryption looks like this: <tpm model='tpm-tis'> <backend type='emulator' version='1.2'> <encryption secret='2c9ceaba-c6ef-4f38-86fd-6e3adb2df5cd'/> </backend> </tpm> The vTPM secret holding the passphrase looks like this: <secret ephemeral='no' private='yes'> <uuid>2c9ceaba-c6ef-4f38-86fd-6e3adb2df5cd</uuid> <description>vTPM passphrase example</description> <usage type='vtpm'> <name>vtpm_example</name> </usage> </secret> The swtpm v0.2 (upcoming) is needed that supports the command line option --print-capabilities returning a JSON object that identifies features added since v0.1. One such features is the possibility to pass a passphrase via a file descriptor. The patches do some refactoring of existing code on the way. Stefan v1->v2: - Added Marc-André's R-bs - Addressed comments - Added patches to extend virCommand to be able to write contents of multiple buffers to file descriptors for a spawned process to read from v2->v3: - Fixed some pointer issues following conversion to use VIR_AUTOFREE v3->v4: - Added test case for virCommandSetSendBuffer() to commantest.c - Addressed other issues raised by Marc-André v4->v5: - Simplified encryption node in TPM's domain XML and adapted everything that depends on this; dropped some patches and removed some R-bs for non-trivial stuff - Not limiting write size for fd's with O_NONBLOCK set Stefan Berger (20): secret: Add support for usage type vTPM, extend schema and test case tests: Add already existing test case tpm-emulator-tpm2 conf: Extend TPM XML parser with encryption support schema: Extend the TPM XML schema with support for encryption tests: Add test for TPM XML encryption parser and formatter tests: Add tests for QEMU command line generation with encrypted TPM tpm: Move qemuTPMEmulatorInit to virTPMEmulatorInit in virtpm.c tpm: Refactor virTPMEmulatorInit to use loop tpm: Check whether previously found executables were updated tpm: Parse the capabilities supported by swtpm and swtpm_setup utils: Implement function to pass a buffer to send via a fd to virCommand utils: Convert pollfd array to be allocated utils: Mark inpipe as non-blocking utils: Extend virCommandProcessIO to include the send buffers tests: Extend command test to transfer large data to process on multiple fds tpm: Use fd to pass password to swtpm_setup and swtpm tpm: Pass migration key passphrase via fd to swtpm tpm: Check TPM XML device configuration changes after edit docs: Extend Secret XML documentation with vtpm usage type docs: Extend TPM docs with new encryption element docs/formatdomain.html.in | 12 + docs/formatsecret.html.in | 61 +++- docs/schemas/domaincommon.rng | 11 + docs/schemas/secret.rng | 10 + include/libvirt/libvirt-secret.h | 1 + src/conf/domain_conf.c | 87 +++++- src/conf/domain_conf.h | 6 + src/conf/secret_conf.c | 13 + src/libvirt_private.syms | 10 + src/qemu/qemu_driver.c | 28 ++ src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_extdevice.h | 3 + src/qemu/qemu_tpm.c | 204 ++++++++------ src/util/vircommand.c | 148 +++++++++- src/util/vircommand.h | 5 + src/util/virsecret.c | 2 +- src/util/virtpm.c | 261 ++++++++++++++++++ src/util/virtpm.h | 23 ++ tests/commandhelper.c | 70 ++++- tests/commandtest.c | 113 ++++++++ .../tpm-emulator-tpm2-enc.x86_64-latest.args | 35 +++ .../tpm-emulator-tpm2-enc.xml | 32 +++ tests/qemuxml2argvtest.c | 1 + .../tpm-emulator-tpm2-enc.xml | 36 +++ tests/qemuxml2xmltest.c | 2 + tests/secretxml2xmlin/usage-vtpm.xml | 7 + tests/secretxml2xmltest.c | 1 + 27 files changed, 1095 insertions(+), 89 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml create mode 100644 tests/secretxml2xmlin/usage-vtpm.xml -- 2.20.1
Add support for usage type vTPM to secret. Extend the schema for the Secret to support the vTPM usage type and add a test case for parsing the Secret with usage type vTPM. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- docs/schemas/secret.rng | 10 ++++++++++ include/libvirt/libvirt-secret.h | 1 + src/conf/secret_conf.c | 13 +++++++++++++ src/util/virsecret.c | 2 +- tests/secretxml2xmlin/usage-vtpm.xml | 7 +++++++ tests/secretxml2xmltest.c | 1 + 6 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/secretxml2xmlin/usage-vtpm.xml diff --git a/docs/schemas/secret.rng b/docs/schemas/secret.rng index 1e94d66e48..e0add8a5e9 100644 --- a/docs/schemas/secret.rng +++ b/docs/schemas/secret.rng @@ -37,6 +37,7 @@ <ref name='usageceph'/> <ref name='usageiscsi'/> <ref name='usagetls'/> + <ref name='usagevtpm'/> <!-- More choices later --> </choice> </element> @@ -81,4 +82,13 @@ </element> </define> + <define name='usagevtpm'> + <attribute name='type'> + <value>vtpm</value> + </attribute> + <element name='name'> + <ref name='genericName'/> + </element> + </define> + </grammar> diff --git a/include/libvirt/libvirt-secret.h b/include/libvirt/libvirt-secret.h index 9a1065f0f3..e5aaac9450 100644 --- a/include/libvirt/libvirt-secret.h +++ b/include/libvirt/libvirt-secret.h @@ -43,6 +43,7 @@ typedef enum { VIR_SECRET_USAGE_TYPE_CEPH = 2, VIR_SECRET_USAGE_TYPE_ISCSI = 3, VIR_SECRET_USAGE_TYPE_TLS = 4, + VIR_SECRET_USAGE_TYPE_VTPM = 5, # ifdef VIR_ENUM_SENTINELS VIR_SECRET_USAGE_TYPE_LAST diff --git a/src/conf/secret_conf.c b/src/conf/secret_conf.c index 5b85a7c0be..b291339e77 100644 --- a/src/conf/secret_conf.c +++ b/src/conf/secret_conf.c @@ -110,6 +110,15 @@ virSecretDefParseUsage(xmlXPathContextPtr ctxt, } break; + case VIR_SECRET_USAGE_TYPE_VTPM: + def->usage_id = virXPathString("string(./usage/name)", ctxt); + if (!def->usage_id) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("vTPM usage specified, but name is missing")); + return -1; + } + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), @@ -257,6 +266,10 @@ virSecretDefFormatUsage(virBufferPtr buf, virBufferEscapeString(buf, "<name>%s</name>\n", def->usage_id); break; + case VIR_SECRET_USAGE_TYPE_VTPM: + virBufferEscapeString(buf, "<name>%s</name>\n", def->usage_id); + break; + default: virReportError(VIR_ERR_INTERNAL_ERROR, _("unexpected secret usage type %d"), diff --git a/src/util/virsecret.c b/src/util/virsecret.c index 854dc72b06..7844a76a56 100644 --- a/src/util/virsecret.c +++ b/src/util/virsecret.c @@ -34,7 +34,7 @@ VIR_LOG_INIT("util.secret"); VIR_ENUM_IMPL(virSecretUsage, VIR_SECRET_USAGE_TYPE_LAST, - "none", "volume", "ceph", "iscsi", "tls", + "none", "volume", "ceph", "iscsi", "tls", "vtpm", ); void diff --git a/tests/secretxml2xmlin/usage-vtpm.xml b/tests/secretxml2xmlin/usage-vtpm.xml new file mode 100644 index 0000000000..5baff3034d --- /dev/null +++ b/tests/secretxml2xmlin/usage-vtpm.xml @@ -0,0 +1,7 @@ +<secret ephemeral='no' private='yes'> + <uuid>aa6c7af2-45a7-477c-85a2-fe86d9f2514e</uuid> + <description>vTPM secret</description> + <usage type='vtpm'> + <name>vTPMvTPMvTPM</name> + </usage> +</secret> diff --git a/tests/secretxml2xmltest.c b/tests/secretxml2xmltest.c index fd93703424..595583346a 100644 --- a/tests/secretxml2xmltest.c +++ b/tests/secretxml2xmltest.c @@ -80,6 +80,7 @@ mymain(void) DO_TEST("usage-ceph"); DO_TEST("usage-iscsi"); DO_TEST("usage-tls"); + DO_TEST("usage-vtpm"); return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE; } -- 2.20.1
Add an already existing test case tpm-emulator-tpm2 to qemuxml2xmltest.c Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- tests/qemuxml2xmltest.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a64b17ac28..a29958ae29 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -653,6 +653,7 @@ mymain(void) DO_TEST("tpm-passthrough", NONE); DO_TEST("tpm-passthrough-crb", NONE); DO_TEST("tpm-emulator", NONE); + DO_TEST("tpm-emulator-tpm2", NONE); DO_TEST("metadata", NONE); DO_TEST("metadata-duplicate", NONE); -- 2.20.1
Extend the TPM device XML parser and XML generator with emulator state encryption support. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 31 ++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3323c9a5b1..7b0de160a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13048,6 +13048,14 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * <tpm model='tpm-tis'> * <backend type='emulator' version='2'/> * </tpm> + * + * Emulator state encryption is supported with the following: + * + * <tpm model='tpm-tis'> + * <backend type='emulator' version='2'> + * <encryption uuid='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + * </backend> + * </tpm> */ static virDomainTPMDefPtr virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, @@ -13062,6 +13070,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) model = NULL; VIR_AUTOFREE(char *) backend = NULL; VIR_AUTOFREE(char *) version = NULL; + VIR_AUTOFREE(char *) secretuuid = NULL; VIR_AUTOFREE(xmlNodePtr *) backends = NULL; if (VIR_ALLOC(def) < 0) @@ -13126,6 +13135,15 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV; break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + secretuuid = virXPathString("string(./backend/encryption/@secret)", ctxt); + if (secretuuid) { + if (virUUIDParse(secretuuid, def->data.emulator.secretuuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to parse secret uuid '%s'"), secretuuid); + goto error; + } + def->data.emulator.hassecretuuid = true; + } break; case VIR_DOMAIN_TPM_TYPE_LAST: goto error; @@ -25949,8 +25967,19 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAddLit(buf, "</backend>\n"); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: - virBufferAsprintf(buf, " version='%s'/>\n", + virBufferAsprintf(buf, " version='%s'", virDomainTPMVersionTypeToString(def->version)); + if (def->data.emulator.hassecretuuid) { + char uuidstr[VIR_UUID_STRING_BUFLEN]; + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + virBufferAsprintf(buf, "<encryption secret='%s'/>\n", + virUUIDFormat(def->data.emulator.secretuuid, uuidstr)); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</backend>\n"); + } else { + virBufferAddLit(buf, "/>\n"); + } break; case VIR_DOMAIN_TPM_TYPE_LAST: break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c1b5fc1337..3facf506ce 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1270,6 +1270,8 @@ struct _virDomainTPMDef { virDomainChrSourceDef source; char *storagepath; char *logfile; + unsigned char secretuuid[VIR_UUID_BUFLEN]; + bool hassecretuuid; } emulator; } data; }; -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:37PM -0400, Stefan Berger wrote:
Extend the TPM device XML parser and XML generator with emulator state encryption support.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 31 ++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 2 ++ 2 files changed, 32 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3323c9a5b1..7b0de160a6 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -13048,6 +13048,14 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * <tpm model='tpm-tis'> * <backend type='emulator' version='2'/> * </tpm> + * + * Emulator state encryption is supported with the following: + * + * <tpm model='tpm-tis'> + * <backend type='emulator' version='2'> + * <encryption uuid='32ee7e76-2178-47a1-ab7b-269e6e348015'/>
Nitpick, you actally parse the attribute called 'secret' instead of 'uuid', which is good. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Extend the TPM XML schema with support for an encryption node. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 31db599ab9..78d72c08bf 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4316,6 +4316,7 @@ <attribute name="type"> <value>emulator</value> </attribute> + <ref name="tpm-backend-emulator-encryption"/> </group> </choice> <choice> @@ -4345,6 +4346,16 @@ </optional> </define> + <define name="tpm-backend-emulator-encryption"> + <optional> + <element name="encryption"> + <attribute name="secret"> + <ref name="UUID"/> + </attribute> + </element> + </optional> + </define> + <define name="vsock"> <element name="vsock"> <optional> -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:38PM -0400, Stefan Berger wrote:
Extend the TPM XML schema with support for an encryption node.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/schemas/domaincommon.rng | 11 +++++++++++ 1 file changed, 11 insertions(+)
Arguably should be squashed into the previous patch. Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Add a test case for the TPM XML encryption parser and formatter. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../tpm-emulator-tpm2-enc.xml | 32 +++++++++++++++++ .../tpm-emulator-tpm2-enc.xml | 36 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 69 insertions(+) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml new file mode 100644 index 0000000000..d889aae4f6 --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml @@ -0,0 +1,32 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <tpm model='tpm-tis'> + <backend type='emulator' version='2.0'> + <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + </backend> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml new file mode 100644 index 0000000000..8902725097 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml @@ -0,0 +1,36 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-i440fx-2.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-x86_64</emulator> + <controller type='usb' index='0'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <controller type='pci' index='0' model='pci-root'/> + <input type='mouse' bus='ps2'/> + <input type='keyboard' bus='ps2'/> + <tpm model='tpm-tis'> + <backend type='emulator' version='2.0'> + <encryption secret='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + </backend> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index a29958ae29..f6cd56cc8f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -654,6 +654,7 @@ mymain(void) DO_TEST("tpm-passthrough-crb", NONE); DO_TEST("tpm-emulator", NONE); DO_TEST("tpm-emulator-tpm2", NONE); + DO_TEST("tpm-emulator-tpm2-enc", NONE); DO_TEST("metadata", NONE); DO_TEST("metadata-duplicate", NONE); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:39PM -0400, Stefan Berger wrote:
Add a test case for the TPM XML encryption parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../tpm-emulator-tpm2-enc.xml | 32 +++++++++++++++++ .../tpm-emulator-tpm2-enc.xml | 36 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 69 insertions(+) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
The QEMU command line does not change when TPM state is encrypted compared to when it is plain. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../tpm-emulator-tpm2-enc.x86_64-latest.args | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 2 files changed, 36 insertions(+) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args new file mode 100644 index 0000000000..3c8dc8e483 --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args @@ -0,0 +1,35 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/tmp/lib/domain--1-TPM-VM \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/tmp/lib/domain--1-TPM-VM/.local/share \ +XDG_CACHE_HOME=/tmp/lib/domain--1-TPM-VM/.cache \ +XDG_CONFIG_HOME=/tmp/lib/domain--1-TPM-VM/.config \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-x86_64 \ +-name guest=TPM-VM,debug-threads=on \ +-S \ +-object secret,id=masterKey0,format=raw,\ +file=/tmp/lib/domain--1-TPM-VM/master-key.aes \ +-machine pc-i440fx-2.12,accel=tcg,usb=off,dump-guest-core=off \ +-m 2048 \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 11d7cd22-da89-3094-6212-079a48a309a1 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot menu=on,strict=on \ +-device piix3-usb-uhci,id=usb,bus=pci.0,addr=0x1.0x2 \ +-tpmdev emulator,id=tpm-tpm0,chardev=chrtpm \ +-chardev socket,id=chrtpm,path=/dev/test \ +-device tpm-tis,tpmdev=tpm-tpm0,id=tpm0 \ +-device virtio-balloon-pci,id=balloon0,bus=pci.0,addr=0x2 \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,\ +resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 91ca35d469..9c02cac8fc 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2082,6 +2082,7 @@ mymain(void) QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); DO_TEST_CAPS_LATEST("tpm-emulator"); DO_TEST_CAPS_LATEST("tpm-emulator-tpm2"); + DO_TEST_CAPS_LATEST("tpm-emulator-tpm2-enc"); DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE); DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:40PM -0400, Stefan Berger wrote:
The QEMU command line does not change when TPM state is encrypted compared to when it is plain.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- .../tpm-emulator-tpm2-enc.x86_64-latest.args | 35 +++++++++++++++++++ tests/qemuxml2argvtest.c | 1 + 2 files changed, 36 insertions(+) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.x86_64-latest.args
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Move qemuTPMEmulatorInit to virTPMEmulatorInit in virtpm.c and introduce a few functions to query the executables needed for virCommands. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/libvirt_private.syms | 4 ++ src/qemu/qemu_tpm.c | 83 ++++-------------------------------- src/util/virtpm.c | 91 ++++++++++++++++++++++++++++++++++++++++ src/util/virtpm.h | 5 +++ 4 files changed, 108 insertions(+), 75 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 34937adc5d..5b3c176862 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3177,6 +3177,10 @@ virTimeStringThenRaw; # util/virtpm.h virTPMCreateCancelPath; +virTPMEmulatorInit; +virTPMGetSwtpm; +virTPMGetSwtpmIoctl; +virTPMGetSwtpmSetup; # util/virtypedparam.h diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index cc8c69433b..61b4f72320 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -41,79 +41,12 @@ #include "configmake.h" #include "dirname.h" #include "qemu_tpm.h" +#include "virtpm.h" #define VIR_FROM_THIS VIR_FROM_NONE VIR_LOG_INIT("qemu.tpm"); -/* - * executables for the swtpm; to be found on the host - */ -static char *swtpm_path; -static char *swtpm_setup; -static char *swtpm_ioctl; - -/* - * qemuTPMEmulatorInit - * - * Initialize the Emulator functions by searching for necessary - * executables that we will use to start and setup the swtpm - */ -static int -qemuTPMEmulatorInit(void) -{ - if (!swtpm_path) { - swtpm_path = virFindFileInPath("swtpm"); - if (!swtpm_path) { - virReportSystemError(ENOENT, "%s", - _("Unable to find 'swtpm' binary in $PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_path)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("TPM emulator %s is not an executable"), - swtpm_path); - VIR_FREE(swtpm_path); - return -1; - } - } - - if (!swtpm_setup) { - swtpm_setup = virFindFileInPath("swtpm_setup"); - if (!swtpm_setup) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find 'swtpm_setup' in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_setup)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("'%s' is not an executable"), - swtpm_setup); - VIR_FREE(swtpm_setup); - return -1; - } - } - - if (!swtpm_ioctl) { - swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); - if (!swtpm_ioctl) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find swtpm_ioctl in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_ioctl)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("swtpm_ioctl program %s is not an executable"), - swtpm_ioctl); - VIR_FREE(swtpm_ioctl); - return -1; - } - } - - return 0; -} - - /* * qemuTPMCreateEmulatorStoragePath * @@ -350,7 +283,7 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir, if (!pidfile) return -ENOMEM; - ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path); + ret = virPidFileReadPathIfAlive(pidfile, pid, virTPMGetSwtpm()); VIR_FREE(pidfile); @@ -386,7 +319,7 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, { int ret = -1; - if (qemuTPMEmulatorInit() < 0) + if (virTPMEmulatorInit() < 0) return -1; /* create log dir ... allow 'tss' user to cd into it */ @@ -478,7 +411,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, "this requires privileged mode for a " "TPM 1.2\n"), 0600); - cmd = virCommandNew(swtpm_setup); + cmd = virCommandNew(virTPMGetSwtpmSetup()); if (!cmd) goto cleanup; @@ -518,7 +451,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not run '%s'. exitstatus: %d; " "Check error log '%s' for details."), - swtpm_setup, exitstatus, logfile); + virTPMGetSwtpmSetup(), exitstatus, logfile); goto cleanup; } @@ -575,7 +508,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, unlink(tpm->data.emulator.source.data.nix.path); - cmd = virCommandNew(swtpm_path); + cmd = virCommandNew(virTPMGetSwtpm()); if (!cmd) goto error; @@ -640,7 +573,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, char *pathname; char *errbuf = NULL; - if (qemuTPMEmulatorInit() < 0) + if (virTPMEmulatorInit() < 0) return; if (!(pathname = qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) @@ -649,7 +582,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, if (!virFileExists(pathname)) goto cleanup; - cmd = virCommandNew(swtpm_ioctl); + cmd = virCommandNew(virTPMGetSwtpmIoctl()); if (!cmd) goto cleanup; diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 583b9a64a4..4635d8add0 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -72,3 +72,94 @@ virTPMCreateCancelPath(const char *devpath) cleanup: return path; } + +/* + * executables for the swtpm; to be found on the host + */ +static char *swtpm_path; +static char *swtpm_setup; +static char *swtpm_ioctl; + +const char * +virTPMGetSwtpm(void) +{ + if (!swtpm_path) + virTPMEmulatorInit(); + return swtpm_path; +} + +const char * +virTPMGetSwtpmSetup(void) +{ + if (!swtpm_setup) + virTPMEmulatorInit(); + return swtpm_setup; +} + +const char * +virTPMGetSwtpmIoctl(void) +{ + if (!swtpm_ioctl) + virTPMEmulatorInit(); + return swtpm_ioctl; +} + +/* + * virTPMEmulatorInit + * + * Initialize the Emulator functions by searching for necessary + * executables that we will use to start and setup the swtpm + */ +int +virTPMEmulatorInit(void) +{ + if (!swtpm_path) { + swtpm_path = virFindFileInPath("swtpm"); + if (!swtpm_path) { + virReportSystemError(ENOENT, "%s", + _("Unable to find 'swtpm' binary in $PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("TPM emulator %s is not an executable"), + swtpm_path); + VIR_FREE(swtpm_path); + return -1; + } + } + + if (!swtpm_setup) { + swtpm_setup = virFindFileInPath("swtpm_setup"); + if (!swtpm_setup) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find 'swtpm_setup' in PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_setup)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s' is not an executable"), + swtpm_setup); + VIR_FREE(swtpm_setup); + return -1; + } + } + + if (!swtpm_ioctl) { + swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); + if (!swtpm_ioctl) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find swtpm_ioctl in PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_ioctl)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("swtpm_ioctl program %s is not an executable"), + swtpm_ioctl); + VIR_FREE(swtpm_ioctl); + return -1; + } + } + + return 0; +} diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 4408bdb217..66d55fb231 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -21,3 +21,8 @@ #pragma once char *virTPMCreateCancelPath(const char *devpath) ATTRIBUTE_NOINLINE; + +const char *virTPMGetSwtpm(void); +const char *virTPMGetSwtpmSetup(void); +const char *virTPMGetSwtpmIoctl(void); +int virTPMEmulatorInit(void); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:41PM -0400, Stefan Berger wrote:
Move qemuTPMEmulatorInit to virTPMEmulatorInit in virtpm.c and introduce a few functions to query the executables needed for virCommands.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/libvirt_private.syms | 4 ++ src/qemu/qemu_tpm.c | 83 ++++-------------------------------- src/util/virtpm.c | 91 ++++++++++++++++++++++++++++++++++++++++ src/util/virtpm.h | 5 +++ 4 files changed, 108 insertions(+), 75 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 34937adc5d..5b3c176862 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3177,6 +3177,10 @@ virTimeStringThenRaw;
# util/virtpm.h virTPMCreateCancelPath; +virTPMEmulatorInit; +virTPMGetSwtpm; +virTPMGetSwtpmIoctl; +virTPMGetSwtpmSetup;
# util/virtypedparam.h diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index cc8c69433b..61b4f72320 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -41,79 +41,12 @@ #include "configmake.h" #include "dirname.h" #include "qemu_tpm.h" +#include "virtpm.h"
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("qemu.tpm");
-/* - * executables for the swtpm; to be found on the host - */ -static char *swtpm_path; -static char *swtpm_setup; -static char *swtpm_ioctl; - -/* - * qemuTPMEmulatorInit - * - * Initialize the Emulator functions by searching for necessary - * executables that we will use to start and setup the swtpm - */ -static int -qemuTPMEmulatorInit(void) -{ - if (!swtpm_path) { - swtpm_path = virFindFileInPath("swtpm"); - if (!swtpm_path) { - virReportSystemError(ENOENT, "%s", - _("Unable to find 'swtpm' binary in $PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_path)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("TPM emulator %s is not an executable"), - swtpm_path); - VIR_FREE(swtpm_path); - return -1; - } - } - - if (!swtpm_setup) { - swtpm_setup = virFindFileInPath("swtpm_setup"); - if (!swtpm_setup) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find 'swtpm_setup' in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_setup)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("'%s' is not an executable"), - swtpm_setup); - VIR_FREE(swtpm_setup); - return -1; - } - } - - if (!swtpm_ioctl) { - swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); - if (!swtpm_ioctl) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find swtpm_ioctl in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_ioctl)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("swtpm_ioctl program %s is not an executable"), - swtpm_ioctl); - VIR_FREE(swtpm_ioctl); - return -1; - } - } - - return 0; -} - - /* * qemuTPMCreateEmulatorStoragePath * @@ -350,7 +283,7 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir, if (!pidfile) return -ENOMEM;
- ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path); + ret = virPidFileReadPathIfAlive(pidfile, pid, virTPMGetSwtpm());
VIR_FREE(pidfile);
@@ -386,7 +319,7 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, { int ret = -1;
- if (qemuTPMEmulatorInit() < 0) + if (virTPMEmulatorInit() < 0) return -1;
/* create log dir ... allow 'tss' user to cd into it */ @@ -478,7 +411,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, "this requires privileged mode for a " "TPM 1.2\n"), 0600);
- cmd = virCommandNew(swtpm_setup); + cmd = virCommandNew(virTPMGetSwtpmSetup()); if (!cmd) goto cleanup;
@@ -518,7 +451,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not run '%s'. exitstatus: %d; " "Check error log '%s' for details."), - swtpm_setup, exitstatus, logfile); + virTPMGetSwtpmSetup(), exitstatus, logfile); goto cleanup; }
@@ -575,7 +508,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm,
unlink(tpm->data.emulator.source.data.nix.path);
- cmd = virCommandNew(swtpm_path); + cmd = virCommandNew(virTPMGetSwtpm()); if (!cmd) goto error;
@@ -640,7 +573,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, char *pathname; char *errbuf = NULL;
- if (qemuTPMEmulatorInit() < 0) + if (virTPMEmulatorInit() < 0) return;
if (!(pathname = qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) @@ -649,7 +582,7 @@ qemuTPMEmulatorStop(const char *swtpmStateDir, if (!virFileExists(pathname)) goto cleanup;
- cmd = virCommandNew(swtpm_ioctl); + cmd = virCommandNew(virTPMGetSwtpmIoctl()); if (!cmd) goto cleanup;
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 583b9a64a4..4635d8add0 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -72,3 +72,94 @@ virTPMCreateCancelPath(const char *devpath) cleanup: return path; } + +/* + * executables for the swtpm; to be found on the host + */ +static char *swtpm_path; +static char *swtpm_setup; +static char *swtpm_ioctl; + +const char * +virTPMGetSwtpm(void) +{ + if (!swtpm_path) + virTPMEmulatorInit();
This isn't a thread safe initialization process. Use VIR_ONCE_GLOBAL_INIT to declare the initializer funtion, then just unconditionally invoke the initializer here. This is then guaranteed to be called only once with no race possible
+ return swtpm_path; +} + +const char * +virTPMGetSwtpmSetup(void) +{ + if (!swtpm_setup) + virTPMEmulatorInit(); + return swtpm_setup; +} + +const char * +virTPMGetSwtpmIoctl(void) +{ + if (!swtpm_ioctl) + virTPMEmulatorInit(); + return swtpm_ioctl; +} + +/* + * virTPMEmulatorInit + * + * Initialize the Emulator functions by searching for necessary + * executables that we will use to start and setup the swtpm + */ +int +virTPMEmulatorInit(void) +{ + if (!swtpm_path) { + swtpm_path = virFindFileInPath("swtpm"); + if (!swtpm_path) { + virReportSystemError(ENOENT, "%s", + _("Unable to find 'swtpm' binary in $PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("TPM emulator %s is not an executable"), + swtpm_path); + VIR_FREE(swtpm_path); + return -1; + } + } + + if (!swtpm_setup) { + swtpm_setup = virFindFileInPath("swtpm_setup"); + if (!swtpm_setup) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find 'swtpm_setup' in PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_setup)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("'%s' is not an executable"), + swtpm_setup); + VIR_FREE(swtpm_setup); + return -1; + } + } + + if (!swtpm_ioctl) { + swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); + if (!swtpm_ioctl) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not find swtpm_ioctl in PATH")); + return -1; + } + if (!virFileIsExecutable(swtpm_ioctl)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("swtpm_ioctl program %s is not an executable"), + swtpm_ioctl); + VIR_FREE(swtpm_ioctl); + return -1; + } + } + + return 0; +} diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 4408bdb217..66d55fb231 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -21,3 +21,8 @@ #pragma once
char *virTPMCreateCancelPath(const char *devpath) ATTRIBUTE_NOINLINE; + +const char *virTPMGetSwtpm(void); +const char *virTPMGetSwtpmSetup(void); +const char *virTPMGetSwtpmIoctl(void); +int virTPMEmulatorInit(void); -- 2.20.1
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/24/19 12:55 PM, Daniel P. Berrangé wrote:
This isn't a thread safe initialization process.
Use VIR_ONCE_GLOBAL_INIT to declare the initializer funtion, then just unconditionally invoke the initializer here. This is then guaranteed to be called only once with no race possible
As you noticed I am intending to call the initialization function searching for the executables multiple times. I have now added the lock to this patch here. Stefan
Refactor virTPMEmulatorInit to use a loop with parameters. This allows for easier extension later on. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virtpm.c | 80 ++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 43 deletions(-) diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 4635d8add0..0179b1e8be 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -113,51 +113,45 @@ virTPMGetSwtpmIoctl(void) int virTPMEmulatorInit(void) { - if (!swtpm_path) { - swtpm_path = virFindFileInPath("swtpm"); - if (!swtpm_path) { - virReportSystemError(ENOENT, "%s", - _("Unable to find 'swtpm' binary in $PATH")); - return -1; + static const struct { + const char *name; + char **path; + } prgs[] = { + { + .name = "swtpm", + .path = &swtpm_path, + }, + { + .name = "swtpm_setup", + .path = &swtpm_setup, + }, + { + .name = "swtpm_ioctl", + .path = &swtpm_ioctl, } - if (!virFileIsExecutable(swtpm_path)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("TPM emulator %s is not an executable"), - swtpm_path); - VIR_FREE(swtpm_path); - return -1; - } - } - - if (!swtpm_setup) { - swtpm_setup = virFindFileInPath("swtpm_setup"); - if (!swtpm_setup) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find 'swtpm_setup' in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_setup)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("'%s' is not an executable"), - swtpm_setup); - VIR_FREE(swtpm_setup); - return -1; - } - } + }; + size_t i; - if (!swtpm_ioctl) { - swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); - if (!swtpm_ioctl) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find swtpm_ioctl in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_ioctl)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("swtpm_ioctl program %s is not an executable"), - swtpm_ioctl); - VIR_FREE(swtpm_ioctl); - return -1; + for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { + char *path; + bool findit = *prgs[i].path == NULL; + + if (findit) { + path = virFindFileInPath(prgs[i].name); + if (!path) { + virReportSystemError(ENOENT, + _("Unable to find '%s' binary in $PATH"), + prgs[i].name); + return -1; + } + if (!virFileIsExecutable(path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s is not an executable"), + path); + VIR_FREE(path); + return -1; + } + *prgs[i].path = path; } } -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:42PM -0400, Stefan Berger wrote:
Refactor virTPMEmulatorInit to use a loop with parameters. This allows for easier extension later on.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/util/virtpm.c | 80 ++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 43 deletions(-)
diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 4635d8add0..0179b1e8be 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -113,51 +113,45 @@ virTPMGetSwtpmIoctl(void) int virTPMEmulatorInit(void) { - if (!swtpm_path) { - swtpm_path = virFindFileInPath("swtpm"); - if (!swtpm_path) { - virReportSystemError(ENOENT, "%s", - _("Unable to find 'swtpm' binary in $PATH")); - return -1; + static const struct { + const char *name; + char **path; + } prgs[] = { + { + .name = "swtpm", + .path = &swtpm_path, + }, + { + .name = "swtpm_setup", + .path = &swtpm_setup, + }, + { + .name = "swtpm_ioctl", + .path = &swtpm_ioctl, } - if (!virFileIsExecutable(swtpm_path)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("TPM emulator %s is not an executable"), - swtpm_path); - VIR_FREE(swtpm_path); - return -1; - } - } - - if (!swtpm_setup) { - swtpm_setup = virFindFileInPath("swtpm_setup"); - if (!swtpm_setup) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find 'swtpm_setup' in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_setup)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("'%s' is not an executable"), - swtpm_setup); - VIR_FREE(swtpm_setup); - return -1; - } - } + }; + size_t i;
- if (!swtpm_ioctl) { - swtpm_ioctl = virFindFileInPath("swtpm_ioctl"); - if (!swtpm_ioctl) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("Could not find swtpm_ioctl in PATH")); - return -1; - } - if (!virFileIsExecutable(swtpm_ioctl)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("swtpm_ioctl program %s is not an executable"), - swtpm_ioctl); - VIR_FREE(swtpm_ioctl); - return -1; + for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { + char *path; + bool findit = *prgs[i].path == NULL; + + if (findit) {
Not seeing the neeed for the 'if(findit)' conditional - especially if you change to use virOnce, this code shold only ever be run once.
+ path = virFindFileInPath(prgs[i].name); + if (!path) { + virReportSystemError(ENOENT, + _("Unable to find '%s' binary in $PATH"), + prgs[i].name); + return -1; + } + if (!virFileIsExecutable(path)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("%s is not an executable"), + path); + VIR_FREE(path); + return -1; + } + *prgs[i].path = path; } }
-- 2.20.1
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Check whether previously found executables were updated and if so look for them again. This helps to use updated features of swtpm and its tools upon updating them. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_tpm.c | 1 + src/util/virtpm.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 61b4f72320..2afa8db448 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -20,6 +20,7 @@ #include <config.h> +#include <sys/stat.h> #include <sys/types.h> #include <unistd.h> #include <fcntl.h> diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 0179b1e8be..e4735f9c4d 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -77,8 +77,13 @@ virTPMCreateCancelPath(const char *devpath) * executables for the swtpm; to be found on the host */ static char *swtpm_path; +static struct stat swtpm_stat; + static char *swtpm_setup; +static struct stat swtpm_setup_stat; + static char *swtpm_ioctl; +static struct stat swtpm_ioctl_stat; const char * virTPMGetSwtpm(void) @@ -116,18 +121,22 @@ virTPMEmulatorInit(void) static const struct { const char *name; char **path; + struct stat *stat; } prgs[] = { { .name = "swtpm", .path = &swtpm_path, + .stat = &swtpm_stat, }, { .name = "swtpm_setup", .path = &swtpm_setup, + .stat = &swtpm_setup_stat, }, { .name = "swtpm_ioctl", .path = &swtpm_ioctl, + .stat = &swtpm_ioctl_stat, } }; size_t i; @@ -135,6 +144,23 @@ virTPMEmulatorInit(void) for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { char *path; bool findit = *prgs[i].path == NULL; + struct stat statbuf; + char *tmp; + + if (!findit) { + /* has executables changed? */ + if (stat(*prgs[i].path, &statbuf) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + findit = true; + } + if (!findit && + memcmp(&statbuf.st_mtim, + &prgs[i].stat->st_mtime, + sizeof(statbuf.st_mtim))) { + findit = true; + } + } if (findit) { path = virFindFileInPath(prgs[i].name); @@ -151,7 +177,15 @@ virTPMEmulatorInit(void) VIR_FREE(path); return -1; } + if (stat(path, prgs[i].stat) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + VIR_FREE(path); + return -1; + } + tmp = *prgs[i].path; *prgs[i].path = path; + VIR_FREE(tmp); } } -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:43PM -0400, Stefan Berger wrote:
Check whether previously found executables were updated and if so look for them again. This helps to use updated features of swtpm and its tools upon updating them.
Hmm, so this answers some of my earlier questions. If we actually intentionally want the initializer to run many times, then we definitely need some explicit locking in here, rather than jutt assuming the QEMU driver has correctly serialized invokation.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- src/qemu/qemu_tpm.c | 1 + src/util/virtpm.c | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 61b4f72320..2afa8db448 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -20,6 +20,7 @@
#include <config.h>
+#include <sys/stat.h> #include <sys/types.h> #include <unistd.h> #include <fcntl.h> diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 0179b1e8be..e4735f9c4d 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -77,8 +77,13 @@ virTPMCreateCancelPath(const char *devpath) * executables for the swtpm; to be found on the host */ static char *swtpm_path; +static struct stat swtpm_stat; + static char *swtpm_setup; +static struct stat swtpm_setup_stat; + static char *swtpm_ioctl; +static struct stat swtpm_ioctl_stat;
const char * virTPMGetSwtpm(void) @@ -116,18 +121,22 @@ virTPMEmulatorInit(void) static const struct { const char *name; char **path; + struct stat *stat; } prgs[] = { { .name = "swtpm", .path = &swtpm_path, + .stat = &swtpm_stat, }, { .name = "swtpm_setup", .path = &swtpm_setup, + .stat = &swtpm_setup_stat, }, { .name = "swtpm_ioctl", .path = &swtpm_ioctl, + .stat = &swtpm_ioctl_stat, } }; size_t i; @@ -135,6 +144,23 @@ virTPMEmulatorInit(void) for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { char *path; bool findit = *prgs[i].path == NULL; + struct stat statbuf; + char *tmp; + + if (!findit) { + /* has executables changed? */ + if (stat(*prgs[i].path, &statbuf) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + findit = true;
If we can't stat the binary, then I we should reset ourselves back to fully uninitialized state. Certainly if we intend to call virReportSystemError, then we must ensure the caller treats it as a fatal error. Currently I believe its still treated as success by the caller since progs[i].path is already non-NULL at this point.
+ } + if (!findit && + memcmp(&statbuf.st_mtim, + &prgs[i].stat->st_mtime, + sizeof(statbuf.st_mtim))) { + findit = true; + } + }
if (findit) { path = virFindFileInPath(prgs[i].name); @@ -151,7 +177,15 @@ virTPMEmulatorInit(void) VIR_FREE(path); return -1; } + if (stat(path, prgs[i].stat) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + VIR_FREE(path); + return -1; + } + tmp = *prgs[i].path; *prgs[i].path = path; + VIR_FREE(tmp); } }
-- 2.20.1
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/24/19 1:02 PM, Daniel P. Berrangé wrote:
On Fri, Jul 12, 2019 at 12:23:43PM -0400, Stefan Berger wrote:
for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { char *path; bool findit = *prgs[i].path == NULL; + struct stat statbuf; + char *tmp; + + if (!findit) { + /* has executables changed? */ + if (stat(*prgs[i].path, &statbuf) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + findit = true; If we can't stat the binary, then I we should reset ourselves back to fully uninitialized state. Certainly if we intend to call virReportSystemError, then we must ensure the caller treats it as a fatal error. Currently I believe its still treated as success by the caller since progs[i].path is already non-NULL at this point.
Removed the virReportError() here since we will try to find it again below and report errors there then.
+ } + if (!findit && + memcmp(&statbuf.st_mtim, + &prgs[i].stat->st_mtime, + sizeof(statbuf.st_mtim))) { + findit = true; + } + }
if (findit) { path = virFindFileInPath(prgs[i].name); @@ -151,7 +177,15 @@ virTPMEmulatorInit(void) VIR_FREE(path); return -1; } + if (stat(path, prgs[i].stat) < 0) { + virReportSystemError(errno, + _("Could not stat %s"), path); + VIR_FREE(path); + return -1; + } + tmp = *prgs[i].path; *prgs[i].path = path; + VIR_FREE(tmp); } }
-- 2.20.1
Regards, Daniel
Run 'swtpm socket --print-capabilities' and 'swtpm_setup --print-capabilities' to get the JSON object of the features the programs are supporting and parse them into a bitmap. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/util/virtpm.c | 134 +++++++++++++++++++++++++++++++++++++-- src/util/virtpm.h | 15 +++++ 3 files changed, 147 insertions(+), 4 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b3c176862..5200adae3f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3181,6 +3181,8 @@ virTPMEmulatorInit; virTPMGetSwtpm; virTPMGetSwtpmIoctl; virTPMGetSwtpmSetup; +virTPMSwtpmFeatureTypeFromString; +virTPMSwtpmSetupFeatureTypeFromString; # util/virtypedparam.h diff --git a/src/util/virtpm.c b/src/util/virtpm.c index e4735f9c4d..692033e899 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -27,9 +27,22 @@ #include "viralloc.h" #include "virfile.h" #include "virtpm.h" +#include "vircommand.h" +#include "virbitmap.h" +#include "virjson.h" #define VIR_FROM_THIS VIR_FROM_NONE +VIR_ENUM_IMPL(virTPMSwtpmFeature, + VIR_TPM_SWTPM_FEATURE_LAST, + "cmdarg-pwd-fd", +); + +VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, + VIR_TPM_SWTPM_SETUP_FEATURE_LAST, + "cmdarg-pwdfile-fd", +); + /** * virTPMCreateCancelPath: * @devpath: Path to the TPM device @@ -74,17 +87,22 @@ virTPMCreateCancelPath(const char *devpath) } /* - * executables for the swtpm; to be found on the host + * executables for the swtpm; to be found on the host along with + * capabilties bitmap */ static char *swtpm_path; static struct stat swtpm_stat; +static virBitmapPtr swtpm_caps; static char *swtpm_setup; static struct stat swtpm_setup_stat; +static virBitmapPtr swtpm_setup_caps; static char *swtpm_ioctl; static struct stat swtpm_ioctl_stat; +typedef int (*TypeFromStringFn)(const char *); + const char * virTPMGetSwtpm(void) { @@ -109,6 +127,99 @@ virTPMGetSwtpmIoctl(void) return swtpm_ioctl; } +/* virTPMExecGetCaps + * + * Execute the prepared command and parse the returned JSON object + * to get the capabilities supported by the executable. + * A JSON object like this is expected: + * + * { + * "type": "swtpm", + * "features": [ + * "cmdarg-seccomp", + * "cmdarg-key-fd", + * "cmdarg-pwd-fd" + * ] + * } + */ +static virBitmapPtr +virTPMExecGetCaps(virCommandPtr cmd, + TypeFromStringFn typeFromStringFn) +{ + int exitstatus; + virBitmapPtr bitmap; + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virJSONValue) json = NULL; + virJSONValuePtr featureList; + virJSONValuePtr item; + size_t idx; + const char *str; + int typ; + + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, &exitstatus) < 0) + return NULL; + + if (!(bitmap = virBitmapNewEmpty())) + return NULL; + + /* older version does not support --print-capabilties -- that's fine */ + if (exitstatus != 0) + return bitmap; + + json = virJSONValueFromString(outbuf); + if (!json) + goto error_bad_json; + + featureList = virJSONValueObjectGetArray(json, "features"); + if (!featureList) + goto error_bad_json; + + if (!virJSONValueIsArray(featureList)) + goto error_bad_json; + + for (idx = 0; idx < virJSONValueArraySize(featureList); idx++) { + item = virJSONValueArrayGet(featureList, idx); + if (!item) + continue; + + str = virJSONValueGetString(item); + if (!str) + goto error_bad_json; + typ = typeFromStringFn(str); + if (typ < 0) + continue; + + if (virBitmapSetBitExpand(bitmap, typ) < 0) + goto cleanup; + } + + cleanup: + return bitmap; + + error_bad_json: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected JSON format: %s"), outbuf); + goto cleanup; +} + +static virBitmapPtr +virTPMGetCaps(TypeFromStringFn typeFromStringFn, + const char *exec, const char *param1) +{ + VIR_AUTOPTR(virCommand) cmd = NULL; + + if (!(cmd = virCommandNew(exec))) + return NULL; + + if (param1) + virCommandAddArg(cmd, param1); + virCommandAddArg(cmd, "--print-capabilities"); + virCommandClearCaps(cmd); + + return virTPMExecGetCaps(cmd, typeFromStringFn); +} + /* * virTPMEmulatorInit * @@ -122,16 +233,24 @@ virTPMEmulatorInit(void) const char *name; char **path; struct stat *stat; + const char *parm; + virBitmapPtr *caps; + TypeFromStringFn typeFromStringFn; } prgs[] = { { .name = "swtpm", .path = &swtpm_path, .stat = &swtpm_stat, + .parm = "socket", + .caps = &swtpm_caps, + .typeFromStringFn = virTPMSwtpmFeatureTypeFromString, }, { .name = "swtpm_setup", .path = &swtpm_setup, .stat = &swtpm_setup_stat, + .caps = &swtpm_setup_caps, + .typeFromStringFn = virTPMSwtpmSetupFeatureTypeFromString, }, { .name = "swtpm_ioctl", @@ -142,7 +261,7 @@ virTPMEmulatorInit(void) size_t i; for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { - char *path; + VIR_AUTOFREE(char *) path = NULL; bool findit = *prgs[i].path == NULL; struct stat statbuf; char *tmp; @@ -174,18 +293,25 @@ virTPMEmulatorInit(void) virReportError(VIR_ERR_INTERNAL_ERROR, _("%s is not an executable"), path); - VIR_FREE(path); return -1; } if (stat(path, prgs[i].stat) < 0) { virReportSystemError(errno, _("Could not stat %s"), path); - VIR_FREE(path); return -1; } tmp = *prgs[i].path; *prgs[i].path = path; VIR_FREE(tmp); + + if (prgs[i].caps) { + *prgs[i].caps = virTPMGetCaps(prgs[i].typeFromStringFn, + path, prgs[i].parm); + path = NULL; + if (!*prgs[i].caps) + return -1; + } + path = NULL; } } diff --git a/src/util/virtpm.h b/src/util/virtpm.h index 66d55fb231..f3dc40cd68 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -26,3 +26,18 @@ const char *virTPMGetSwtpm(void); const char *virTPMGetSwtpmSetup(void); const char *virTPMGetSwtpmIoctl(void); int virTPMEmulatorInit(void); + +typedef enum { + VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD, + + VIR_TPM_SWTPM_FEATURE_LAST +} virTPMSwtpmFeature; + +typedef enum { + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD, + + VIR_TPM_SWTPM_SETUP_FEATURE_LAST +} virTPMSwtpmSetupFeature; + +VIR_ENUM_DECL(virTPMSwtpmFeature); +VIR_ENUM_DECL(virTPMSwtpmSetupFeature); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:44PM -0400, Stefan Berger wrote:
Run 'swtpm socket --print-capabilities' and 'swtpm_setup --print-capabilities' to get the JSON object of the features the programs are supporting and parse them into a bitmap.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/util/virtpm.c | 134 +++++++++++++++++++++++++++++++++++++-- src/util/virtpm.h | 15 +++++ 3 files changed, 147 insertions(+), 4 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5b3c176862..5200adae3f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3181,6 +3181,8 @@ virTPMEmulatorInit; virTPMGetSwtpm; virTPMGetSwtpmIoctl; virTPMGetSwtpmSetup; +virTPMSwtpmFeatureTypeFromString; +virTPMSwtpmSetupFeatureTypeFromString;
# util/virtypedparam.h diff --git a/src/util/virtpm.c b/src/util/virtpm.c index e4735f9c4d..692033e899 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -27,9 +27,22 @@ #include "viralloc.h" #include "virfile.h" #include "virtpm.h" +#include "vircommand.h" +#include "virbitmap.h" +#include "virjson.h"
#define VIR_FROM_THIS VIR_FROM_NONE
+VIR_ENUM_IMPL(virTPMSwtpmFeature, + VIR_TPM_SWTPM_FEATURE_LAST, + "cmdarg-pwd-fd", +); + +VIR_ENUM_IMPL(virTPMSwtpmSetupFeature, + VIR_TPM_SWTPM_SETUP_FEATURE_LAST, + "cmdarg-pwdfile-fd", +); + /** * virTPMCreateCancelPath: * @devpath: Path to the TPM device @@ -74,17 +87,22 @@ virTPMCreateCancelPath(const char *devpath) }
/* - * executables for the swtpm; to be found on the host + * executables for the swtpm; to be found on the host along with + * capabilties bitmap */ static char *swtpm_path; static struct stat swtpm_stat; +static virBitmapPtr swtpm_caps;
static char *swtpm_setup; static struct stat swtpm_setup_stat; +static virBitmapPtr swtpm_setup_caps;
static char *swtpm_ioctl; static struct stat swtpm_ioctl_stat;
+typedef int (*TypeFromStringFn)(const char *); + const char * virTPMGetSwtpm(void) { @@ -109,6 +127,99 @@ virTPMGetSwtpmIoctl(void) return swtpm_ioctl; }
+/* virTPMExecGetCaps + * + * Execute the prepared command and parse the returned JSON object + * to get the capabilities supported by the executable. + * A JSON object like this is expected: + * + * { + * "type": "swtpm", + * "features": [ + * "cmdarg-seccomp", + * "cmdarg-key-fd", + * "cmdarg-pwd-fd" + * ] + * } + */ +static virBitmapPtr +virTPMExecGetCaps(virCommandPtr cmd, + TypeFromStringFn typeFromStringFn) +{ + int exitstatus; + virBitmapPtr bitmap; + VIR_AUTOFREE(char *) outbuf = NULL; + VIR_AUTOPTR(virJSONValue) json = NULL; + virJSONValuePtr featureList; + virJSONValuePtr item; + size_t idx; + const char *str; + int typ; + + virCommandSetOutputBuffer(cmd, &outbuf); + if (virCommandRun(cmd, &exitstatus) < 0) + return NULL; + + if (!(bitmap = virBitmapNewEmpty())) + return NULL; + + /* older version does not support --print-capabilties -- that's fine */ + if (exitstatus != 0) + return bitmap;
We can't distinguish from an old version that lacks --print-capabilties and from a new version that crashed when running --print-capabilties. The latter shouldn't happen in general, but I thin kwe ought to at least put a VIR_DEBUG in here to make it obvious we're treating this as if we hit an old binary when looking at logs.
+ + json = virJSONValueFromString(outbuf); + if (!json) + goto error_bad_json; + + featureList = virJSONValueObjectGetArray(json, "features"); + if (!featureList) + goto error_bad_json; + + if (!virJSONValueIsArray(featureList)) + goto error_bad_json; + + for (idx = 0; idx < virJSONValueArraySize(featureList); idx++) { + item = virJSONValueArrayGet(featureList, idx); + if (!item) + continue; + + str = virJSONValueGetString(item); + if (!str) + goto error_bad_json; + typ = typeFromStringFn(str); + if (typ < 0) + continue; + + if (virBitmapSetBitExpand(bitmap, typ) < 0) + goto cleanup; + } + + cleanup: + return bitmap; + + error_bad_json: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected JSON format: %s"), outbuf); + goto cleanup; +}
@@ -142,7 +261,7 @@ virTPMEmulatorInit(void) size_t i;
for (i = 0; i < ARRAY_CARDINALITY(prgs); i++) { - char *path; + VIR_AUTOFREE(char *) path = NULL; bool findit = *prgs[i].path == NULL; struct stat statbuf; char *tmp; @@ -174,18 +293,25 @@ virTPMEmulatorInit(void) virReportError(VIR_ERR_INTERNAL_ERROR, _("%s is not an executable"), path); - VIR_FREE(path); return -1; } if (stat(path, prgs[i].stat) < 0) { virReportSystemError(errno, _("Could not stat %s"), path); - VIR_FREE(path); return -1; }
If you re-post this series, just use VIR_AUTOFREE straightaway in the earlier patch. With the debug line added Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/24/19 1:05 PM, Daniel P. Berrangé wrote:
On Fri, Jul 12, 2019 at 12:23:44PM -0400, Stefan Berger wrote:
If you re-post this series, just use VIR_AUTOFREE straightaway in the earlier patch.
Done.
With the debug line added
Done
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
I'll post v6 some time tomorrow... but if you get the time to continue on v5, please do.
Regards, Daniel
Thanks!! Stefan
Implement virCommandSetSendBuffer() that allows the caller to pass a file descriptor and buffer to virCommand. virCommand will write the buffer into the file descriptor. That file descriptor could be the write end of a pipe or one of the file descriptors of a socketpair. The other file descriptor should be passed to the launched process to read the data from. Only implement the function to allocate memory for send buffers and to free them later on. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 76 ++++++++++++++++++++++++++++++++++++++++ src/util/vircommand.h | 5 +++ 3 files changed, 82 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 5200adae3f..13829563e9 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1722,6 +1722,7 @@ virCommandSetOutputFD; virCommandSetPidFile; virCommandSetPreExecHook; virCommandSetSELinuxLabel; +virCommandSetSendBuffer; virCommandSetUID; virCommandSetUmask; virCommandSetWorkingDirectory; diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 8695c98d1b..9972a491d6 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -77,6 +77,16 @@ struct _virCommandFD { unsigned int flags; }; +typedef struct _virCommandSendBuffer virCommandSendBuffer; +typedef virCommandSendBuffer *virCommandSendBufferPtr; + +struct _virCommandSendBuffer { + int fd; + unsigned char *buffer; + size_t buflen; + off_t offset; +}; + struct _virCommand { int has_error; /* ENOMEM on allocation failure, -1 for anything else. */ @@ -136,6 +146,9 @@ struct _virCommand { char *appArmorProfile; #endif int mask; + + virCommandSendBufferPtr sendBuffers; + size_t numSendBuffers; }; /* See virCommandSetDryRun for description for this variable */ @@ -1741,6 +1754,67 @@ virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd) } +static int +virCommandGetNumSendBuffers(virCommandPtr cmd) +{ + return cmd->numSendBuffers; +} + + +static void +virCommandFreeSendBuffers(virCommandPtr cmd) +{ + size_t i; + + for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd); + VIR_FREE(cmd->sendBuffers[i].buffer); + } + VIR_FREE(cmd->sendBuffers); +} + + +/** + * virCommandSetSendBuffer + * @cmd: the command to modify + * + * Pass a buffer to virCommand that will be written into the + * given file descriptor. The buffer will be freed automatically + * and the file descriptor closed. + */ +int +virCommandSetSendBuffer(virCommandPtr cmd, + int fd, + unsigned char *buffer, size_t buflen) +{ + size_t i = virCommandGetNumSendBuffers(cmd); + + if (!cmd || cmd->has_error) + return -1; + + if (fcntl(fd, F_SETFL, O_NONBLOCK) < 0) { + virReportSystemError(errno, "%s", + _("fcntl failed to set O_NONBLOCK")); + cmd->has_error = errno; + return -1; + } + + if (VIR_REALLOC_N(cmd->sendBuffers, i + 1) < 0) { + cmd->has_error = ENOMEM; + return -1; + } + + cmd->sendBuffers[i].fd = fd; + cmd->sendBuffers[i].buffer = buffer; + cmd->sendBuffers[i].buflen = buflen; + cmd->sendBuffers[i].offset = 0; + + cmd->numSendBuffers++; + + return 0; +} + + /** * virCommandSetInputBuffer: * @cmd: the command to modify @@ -2880,6 +2954,8 @@ virCommandFree(virCommandPtr cmd) VIR_FREE(cmd->appArmorProfile); #endif + virCommandFreeSendBuffers(cmd); + VIR_FREE(cmd); } diff --git a/src/util/vircommand.h b/src/util/vircommand.h index c9a8d3c41c..716e84af3d 100644 --- a/src/util/vircommand.h +++ b/src/util/vircommand.h @@ -148,6 +148,11 @@ void virCommandAddArgList(virCommandPtr cmd, void virCommandSetWorkingDirectory(virCommandPtr cmd, const char *pwd) ATTRIBUTE_NONNULL(2); +int virCommandSetSendBuffer(virCommandPtr cmd, + int fd, + unsigned char *buffer, size_t buflen) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3); + void virCommandSetInputBuffer(virCommandPtr cmd, const char *inbuf) ATTRIBUTE_NONNULL(2); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:45PM -0400, Stefan Berger wrote:
Implement virCommandSetSendBuffer() that allows the caller to pass a file descriptor and buffer to virCommand. virCommand will write the buffer into the file descriptor. That file descriptor could be the write end of a pipe or one of the file descriptors of a socketpair. The other file descriptor should be passed to the launched process to read the data from.
Only implement the function to allocate memory for send buffers and to free them later on.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 1 + src/util/vircommand.c | 76 ++++++++++++++++++++++++++++++++++++++++ src/util/vircommand.h | 5 +++ 3 files changed, 82 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Convert the struct pollfd *fds to be allocated rather than residing on the stack. This prepares it for the next patch where the size of the array of fds becomes dynamic. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> --- src/util/vircommand.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 9972a491d6..49347a9800 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -2137,6 +2137,7 @@ virCommandProcessIO(virCommandPtr cmd) size_t inlen = 0, outlen = 0, errlen = 0; size_t inoff = 0; int ret = 0; + VIR_AUTOFREE(struct pollfd *) fds = NULL; if (dryRunBuffer || dryRunCallback) { VIR_DEBUG("Dry run requested, skipping I/O processing"); @@ -2168,9 +2169,11 @@ virCommandProcessIO(virCommandPtr cmd) goto cleanup; ret = -1; + if (VIR_ALLOC_N(fds, 3) < 0) + goto cleanup; + for (;;) { size_t i; - struct pollfd fds[3]; int nfds = 0; if (cmd->inpipe != -1) { -- 2.20.1
Mark a virCommand's inpipe (write-end of pipe) as non-blocking so that it will never block when we were to try to write too many bytes to it while it doesn't have the capacity to hold them. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 49347a9800..bb9ecdf348 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -2533,6 +2533,13 @@ virCommandRunAsync(virCommandPtr cmd, pid_t *pid) } cmd->infd = infd[0]; cmd->inpipe = infd[1]; + if (fcntl(cmd->inpipe, F_SETFL, O_NONBLOCK) < 0) { + virReportSystemError(errno, "%s", + _("fcntl failed to set O_NONBLOCK")); + cmd->has_error = -1; + ret = -1; + goto cleanup; + } } else if ((cmd->inbuf && cmd->infd == -1) || (cmd->outbuf && cmd->outfdptr != &cmd->outfd) || (cmd->errbuf && cmd->errfdptr != &cmd->errfd)) { -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:47PM -0400, Stefan Berger wrote:
Mark a virCommand's inpipe (write-end of pipe) as non-blocking so that it will never block when we were to try to write too many bytes to it while it doesn't have the capacity to hold them.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.c | 7 +++++++ 1 file changed, 7 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Extend virCommandProcessIO to include the send buffers in the poll loop. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.c | 62 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index bb9ecdf348..976231444e 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1815,6 +1815,61 @@ virCommandSetSendBuffer(virCommandPtr cmd, } +static int +virCommandSendBuffersFillPollfd(virCommandPtr cmd, + struct pollfd *fds, + int startidx) +{ + size_t i, j; + + for (i = 0, j = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + if (cmd->sendBuffers[i].fd >= 0) { + fds[startidx + j].fd = cmd->sendBuffers[i].fd; + fds[startidx + j].events = POLLOUT; + fds[startidx + j].revents = 0; + j++; + } + } + + return j; +} + + +static int +virCommandSendBuffersHandlePoll(virCommandPtr cmd, + struct pollfd *fds) +{ + size_t i; + int done; + + for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + if (fds->fd == cmd->sendBuffers[i].fd) + break; + } + if (i == virCommandGetNumSendBuffers(cmd)) + return 0; + + done = write(fds->fd, + cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset, + cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset); + if (done < 0) { + if (errno == EPIPE) { + VIR_DEBUG("child closed PIPE early, ignoring EPIPE " + "on fd %d", cmd->sendBuffers[i].fd); + VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd); + } else if (errno != EINTR && errno != EAGAIN) { + virReportSystemError(errno, "%s", + _("unable to write to child input")); + return -1; + } + } else { + cmd->sendBuffers[i].offset += done; + if (cmd->sendBuffers[i].offset == cmd->sendBuffers[i].buflen) + VIR_FORCE_CLOSE(cmd->sendBuffers[i].fd); + } + return 0; +} + /** * virCommandSetInputBuffer: * @cmd: the command to modify @@ -2169,7 +2224,7 @@ virCommandProcessIO(virCommandPtr cmd) goto cleanup; ret = -1; - if (VIR_ALLOC_N(fds, 3) < 0) + if (VIR_ALLOC_N(fds, 3 + virCommandGetNumSendBuffers(cmd)) < 0) goto cleanup; for (;;) { @@ -2195,6 +2250,8 @@ virCommandProcessIO(virCommandPtr cmd) nfds++; } + nfds += virCommandSendBuffersFillPollfd(cmd, fds, nfds); + if (nfds == 0) break; @@ -2267,6 +2324,9 @@ virCommandProcessIO(virCommandPtr cmd) if (inoff == inlen) VIR_FORCE_CLOSE(cmd->inpipe); } + } else if (fds[i].revents & (POLLOUT | POLLHUP | POLLERR)) { + if (virCommandSendBuffersHandlePoll(cmd, &fds[i]) < 0) + goto cleanup; } } } -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:48PM -0400, Stefan Berger wrote:
Extend virCommandProcessIO to include the send buffers in the poll loop.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.c | 62 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index bb9ecdf348..976231444e 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1815,6 +1815,61 @@ virCommandSetSendBuffer(virCommandPtr cmd, }
+static int +virCommandSendBuffersFillPollfd(virCommandPtr cmd, + struct pollfd *fds, + int startidx) +{ + size_t i, j; + + for (i = 0, j = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + if (cmd->sendBuffers[i].fd >= 0) { + fds[startidx + j].fd = cmd->sendBuffers[i].fd; + fds[startidx + j].events = POLLOUT; + fds[startidx + j].revents = 0; + j++; + } + } + + return j; +} + + +static int +virCommandSendBuffersHandlePoll(virCommandPtr cmd, + struct pollfd *fds) +{ + size_t i; + int done;
ssize_t is the return type for write()
+ + for (i = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + if (fds->fd == cmd->sendBuffers[i].fd) + break; + } + if (i == virCommandGetNumSendBuffers(cmd)) + return 0; + + done = write(fds->fd, + cmd->sendBuffers[i].buffer + cmd->sendBuffers[i].offset, + cmd->sendBuffers[i].buflen - cmd->sendBuffers[i].offset);
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/25/19 5:48 AM, Daniel P. Berrangé wrote:
On Fri, Jul 12, 2019 at 12:23:48PM -0400, Stefan Berger wrote:
Extend virCommandProcessIO to include the send buffers in the poll loop.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/util/vircommand.c | 62 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-)
diff --git a/src/util/vircommand.c b/src/util/vircommand.c index bb9ecdf348..976231444e 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -1815,6 +1815,61 @@ virCommandSetSendBuffer(virCommandPtr cmd, }
+static int +virCommandSendBuffersFillPollfd(virCommandPtr cmd, + struct pollfd *fds, + int startidx) +{ + size_t i, j; + + for (i = 0, j = 0; i < virCommandGetNumSendBuffers(cmd); i++) { + if (cmd->sendBuffers[i].fd >= 0) { + fds[startidx + j].fd = cmd->sendBuffers[i].fd; + fds[startidx + j].events = POLLOUT; + fds[startidx + j].revents = 0; + j++; + } + } + + return j; +} + + +static int +virCommandSendBuffersHandlePoll(virCommandPtr cmd, + struct pollfd *fds) +{ + size_t i; + int done; ssize_t is the return type for write()
Fixed. Regards, Stefan
Add a test case to commandtest.c to test the transfer of data to a process who received the read-end of pipes' file descriptors. Transfer large (128 kb) byte streams. Extend the commandhelper.c with support for --readfd <fd> command line parameter and convert the data receive loop to use poll and receive data on multiple file descriptors (up to 3) and read data into distinct buffers that we grow while adding more (string) data. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- tests/commandhelper.c | 70 +++++++++++++++++++++++--- tests/commandtest.c | 113 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 6 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 32ebeeaef2..1312f3ee52 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -23,6 +23,7 @@ #include <unistd.h> #include <fcntl.h> #include <sys/stat.h> +#include <poll.h> #include "internal.h" #define NO_LIBVIRT @@ -62,13 +63,27 @@ int main(int argc, char **argv) { char *cwd; FILE *log = fopen(abs_builddir "/commandhelper.log", "w"); int ret = EXIT_FAILURE; + int readfds[3] = { STDIN_FILENO, }; + int numreadfds = 1; + struct pollfd fds[3]; + int numpollfds = 0; + char *buffers[3] = {NULL, NULL, NULL}; + size_t buflen[3] = {0, 0, 0}; + char c; if (!log) return ret; - for (i = 1; i < argc; i++) + for (i = 1; i < argc; i++) { fprintf(log, "ARG:%s\n", argv[i]); + if (STREQ(argv[i - 1], "--readfd") && + sscanf(argv[i], "%u%c", &readfds[numreadfds++], &c) != 1) { + printf("Could not parse fd %s\n", argv[i]); + goto cleanup; + } + } + origenv = environ; n = 0; while (*origenv != NULL) { @@ -134,15 +149,56 @@ int main(int argc, char **argv) { fprintf(stderr, "BEGIN STDERR\n"); fflush(stderr); + for (i = 0; i < numreadfds; i++) { + fds[numpollfds].fd = readfds[i]; + fds[numpollfds].events = POLLIN; + fds[numpollfds].revents = 0; + numpollfds++; + } + for (;;) { - got = read(STDIN_FILENO, buf, sizeof(buf)); - if (got < 0) + unsigned ctr = 0; + + if (poll(fds, numpollfds, -1) < 0) { + printf("poll failed: %s\n", strerror(errno)); goto cleanup; - if (got == 0) + } + + for (i = 0; i < numpollfds; i++) { + if (fds[i].revents & (POLLIN | POLLHUP | POLLERR)) { + fds[i].revents = 0; + + got = read(fds[i].fd, buf, sizeof(buf)); + if (got < 0) + goto cleanup; + if (got == 0) { + /* do not want to hear from this fd anymore */ + fds[i].events = 0; + } else { + buffers[i] = realloc(buffers[i], buflen[i] + got); + if (!buf[i]) { + fprintf(stdout, "Out of memory!\n"); + goto cleanup; + } + memcpy(buffers[i] + buflen[i], buf, got); + buflen[i] += got; + } + } + } + for (i = 0; i < numpollfds; i++) { + if (fds[i].events) { + ctr++; + break; + } + } + if (ctr == 0) break; - if (write(STDOUT_FILENO, buf, got) != got) + } + + for (i = 0; i < numpollfds; i++) { + if (write(STDOUT_FILENO, buffers[i], buflen[i]) != buflen[i]) goto cleanup; - if (write(STDERR_FILENO, buf, got) != got) + if (write(STDERR_FILENO, buffers[i], buflen[i]) != buflen[i]) goto cleanup; } @@ -154,6 +210,8 @@ int main(int argc, char **argv) { ret = EXIT_SUCCESS; cleanup: + for (i = 0; i < ARRAY_CARDINALITY(buffers); i++) + free(buffers[i]); fclose(log); free(newenv); return ret; diff --git a/tests/commandtest.c b/tests/commandtest.c index 146cc4c1bf..ecabbd3ba4 100644 --- a/tests/commandtest.c +++ b/tests/commandtest.c @@ -1196,6 +1196,118 @@ static int test26(const void *unused ATTRIBUTE_UNUSED) return ret; } +static int test27(const void *unused ATTRIBUTE_UNUSED) +{ + virCommandPtr cmd = virCommandNew(abs_builddir "/commandhelper"); + int pipe1[2]; + int pipe2[2]; + int ret = -1; + size_t buflen = 1024 * 128; + char *buffer0 = NULL; + char *buffer1 = NULL; + char *buffer2 = NULL; + char *outactual = NULL; + char *erractual = NULL; + char *outexpect = NULL; +# define TEST27_OUTEXPECT_TEMP "BEGIN STDOUT\n" \ + "%s%s%s" \ + "END STDOUT\n" + char *errexpect = NULL; +# define TEST27_ERREXPECT_TEMP "BEGIN STDERR\n" \ + "%s%s%s" \ + "END STDERR\n" + + if (VIR_ALLOC_N(buffer0, buflen) < 0 || + VIR_ALLOC_N(buffer1, buflen) < 0 || + VIR_ALLOC_N(buffer2, buflen) < 0) + goto cleanup; + + memset(buffer0, 'H', buflen - 2); + buffer0[buflen - 2] = '\n'; + buffer0[buflen - 1] = 0; + + memset(buffer1, '1', buflen - 2); + buffer1[buflen - 2] = '\n'; + buffer1[buflen - 1] = 0; + + memset(buffer2, '2', buflen - 2); + buffer2[buflen - 2] = '\n'; + buffer2[buflen - 1] = 0; + + if (virAsprintf(&outexpect, TEST27_OUTEXPECT_TEMP, + buffer0, buffer1, buffer2) < 0 || + virAsprintf(&errexpect, TEST27_ERREXPECT_TEMP, + buffer0, buffer1, buffer2) < 0) { + printf("Could not virAsprintf expected output\n"); + goto cleanup; + } + + if (pipe(pipe1) < 0 || pipe(pipe2) < 0) { + printf("Could not create pipe: %s\n", strerror(errno)); + goto cleanup; + } + + if (virCommandSetSendBuffer(cmd, pipe1[1], + (unsigned char *)buffer1, buflen - 1) < 0 || + virCommandSetSendBuffer(cmd, pipe2[1], + (unsigned char *)buffer2, buflen - 1) < 0) { + printf("Could not set send buffers\n"); + goto cleanup; + } + pipe1[1] = 0; + pipe2[1] = 0; + buffer1 = NULL; + buffer2 = NULL; + + virCommandAddArg(cmd, "--readfd"); + virCommandAddArgFormat(cmd, "%d", pipe1[0]); + virCommandPassFD(cmd, pipe1[0], 0); + + virCommandAddArg(cmd, "--readfd"); + virCommandAddArgFormat(cmd, "%d", pipe2[0]); + virCommandPassFD(cmd, pipe2[0], 0); + + virCommandSetInputBuffer(cmd, buffer0); + virCommandSetOutputBuffer(cmd, &outactual); + virCommandSetErrorBuffer(cmd, &erractual); + + if (virCommandRun(cmd, NULL) < 0) { + printf("Cannot run child %s\n", virGetLastErrorMessage()); + goto cleanup; + } + + virCommandFree(cmd); + + if (!outactual || !erractual) + goto cleanup; + + if (STRNEQ(outactual, outexpect)) { + virTestDifference(stderr, outexpect, outactual); + goto cleanup; + } + if (STRNEQ(erractual, errexpect)) { + virTestDifference(stderr, errexpect, erractual); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FORCE_CLOSE(pipe1[0]); + VIR_FORCE_CLOSE(pipe2[0]); + VIR_FORCE_CLOSE(pipe1[1]); + VIR_FORCE_CLOSE(pipe2[1]); + VIR_FREE(buffer0); + VIR_FREE(buffer1); + VIR_FREE(buffer2); + VIR_FREE(outactual); + VIR_FREE(erractual); + VIR_FREE(outexpect); + VIR_FREE(errexpect); + + return ret; +} + static void virCommandThreadWorker(void *opaque) { virCommandTestDataPtr test = opaque; @@ -1350,6 +1462,7 @@ mymain(void) DO_TEST(test24); DO_TEST(test25); DO_TEST(test26); + DO_TEST(test27); virMutexLock(&test->lock); if (test->running) { -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:49PM -0400, Stefan Berger wrote:
Add a test case to commandtest.c to test the transfer of data to a process who received the read-end of pipes' file descriptors. Transfer large (128 kb) byte streams.
Extend the commandhelper.c with support for --readfd <fd> command line parameter and convert the data receive loop to use poll and receive data on multiple file descriptors (up to 3) and read data into distinct buffers that we grow while adding more (string) data.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- tests/commandhelper.c | 70 +++++++++++++++++++++++--- tests/commandtest.c | 113 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 177 insertions(+), 6 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
@@ -62,13 +63,27 @@ int main(int argc, char **argv) { char *cwd; FILE *log = fopen(abs_builddir "/commandhelper.log", "w"); int ret = EXIT_FAILURE; + int readfds[3] = { STDIN_FILENO, }; + int numreadfds = 1; + struct pollfd fds[3]; + int numpollfds = 0;
s/int/size_t/ for array sizes
+ char *buffers[3] = {NULL, NULL, NULL}; + size_t buflen[3] = {0, 0, 0}; + char c;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Allow vTPM state encryption when swtpm_setup and swtpm support passing a passphrase using a file descriptor. This patch enables the encryption of the vTPM state only. It does not encrypt the state during migration, so the destination secret does not need to have the same password at this point. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 110 ++++++++++++++++++++++++++++++++++++++- src/util/virtpm.c | 16 ++++++ src/util/virtpm.h | 3 ++ 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 13829563e9..b531ab1efa 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3182,7 +3182,9 @@ virTPMEmulatorInit; virTPMGetSwtpm; virTPMGetSwtpmIoctl; virTPMGetSwtpmSetup; +virTPMSwtpmCapsGet; virTPMSwtpmFeatureTypeFromString; +virTPMSwtpmSetupCapsGet; virTPMSwtpmSetupFeatureTypeFromString; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 2afa8db448..3bec68bce6 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -43,6 +43,7 @@ #include "dirname.h" #include "qemu_tpm.h" #include "virtpm.h" +#include "secret_util.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -372,6 +373,66 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, return ret; } +/* + * qemuTPMSetupEncryption + * + * @secretuuid: The UUID with the secret holding passphrase + * @cmd: the virCommand to transfer the secret to + * + * Returns file descriptor representing the read-end of a pipe. + * The passphrase can be read from this pipe. Returns < 0 in case + * of error. + * + * This function reads the passphrase and writes it into the + * write-end of a pipe so that the read-end of the pipe can be + * passed to the emulator for reading the passphrase from. + */ +static int +qemuTPMSetupEncryption(const unsigned char *secretuuid, + virCommandPtr cmd) +{ + int ret = -1; + int pipefd[2] = { -1, -1 }; + virConnectPtr conn; + VIR_AUTOFREE(uint8_t *) secret = NULL; + size_t secret_len; + virSecretLookupTypeDef seclookupdef = { + .type = VIR_SECRET_LOOKUP_TYPE_UUID, + }; + + conn = virGetConnectSecret(); + if (!conn) + return -1; + + memcpy(seclookupdef.u.uuid, secretuuid, sizeof(seclookupdef.u.uuid)); + if (virSecretGetSecretString(conn, &seclookupdef, + VIR_SECRET_USAGE_TYPE_VTPM, + &secret, &secret_len) < 0) + goto error; + + if (pipe(pipefd) == -1) { + virReportSystemError(errno, "%s", + _("Unable to create pipe")); + goto error; + } + + if (virCommandSetSendBuffer(cmd, pipefd[1], secret, secret_len) < 0) + goto error; + + secret = NULL; + ret = pipefd[0]; + + cleanup: + virObjectUnref(conn); + + return ret; + + error: + VIR_FORCE_CLOSE(pipefd[1]); + VIR_FORCE_CLOSE(pipefd[0]); + + goto cleanup; +} /* * qemuTPMEmulatorRunSetup @@ -386,6 +447,7 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, * @logfile: The file to write the log into; it must be writable * for the user given by userid or 'tss' * @tpmversion: The version of the TPM, either a TPM 1.2 or TPM 2 + * @encryption: pointer to virStorageEncryption holding secret * * Setup the external swtpm by creating endorsement key and * certificates for it. @@ -398,13 +460,15 @@ qemuTPMEmulatorRunSetup(const char *storagepath, uid_t swtpm_user, gid_t swtpm_group, const char *logfile, - const virDomainTPMVersion tpmversion) + const virDomainTPMVersion tpmversion, + const unsigned char *secretuuid) { virCommandPtr cmd = NULL; int exitstatus; int ret = -1; char uuid[VIR_UUID_STRING_BUFLEN]; char *vmid = NULL; + VIR_AUTOCLOSE pwdfile_fd = -1; if (!privileged && tpmversion == VIR_DOMAIN_TPM_VERSION_1_2) return virFileWriteStr(logfile, @@ -434,6 +498,23 @@ qemuTPMEmulatorRunSetup(const char *storagepath, break; } + if (secretuuid) { + if (!virTPMSwtpmSetupCapsGet( + VIR_TPM_SWTPM_SETUP_FEATURE_CMDARG_PWDFILE_FD)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%s does not support passing a passphrase using a file " + "descriptor"), virTPMGetSwtpmSetup()); + goto cleanup; + } + if ((pwdfile_fd = qemuTPMSetupEncryption(secretuuid, cmd)) < 0) + goto cleanup; + + virCommandAddArg(cmd, "--pwdfile-fd"); + virCommandAddArgFormat(cmd, "%d", pwdfile_fd); + virCommandAddArgList(cmd, "--cipher", "aes-256-cbc", NULL); + virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pwdfile_fd = -1; + } virCommandAddArgList(cmd, "--tpm-state", storagepath, @@ -496,15 +577,21 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, virCommandPtr cmd = NULL; bool created = false; char *pidfile; + VIR_AUTOCLOSE pwdfile_fd = -1; + const unsigned char *secretuuid = NULL; if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, &created, swtpm_user, swtpm_group) < 0) return NULL; + if (tpm->data.emulator.hassecretuuid) + secretuuid = tpm->data.emulator.secretuuid; + if (created && qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid, privileged, swtpm_user, swtpm_group, - tpm->data.emulator.logfile, tpm->version) < 0) + tpm->data.emulator.logfile, tpm->version, + secretuuid) < 0) goto error; unlink(tpm->data.emulator.source.data.nix.path); @@ -547,6 +634,25 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, virCommandAddArgFormat(cmd, "file=%s", pidfile); VIR_FREE(pidfile); + if (tpm->data.emulator.hassecretuuid) { + if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, + _("%s does not support passing passphrase via file descriptor"), + virTPMGetSwtpm()); + goto error; + } + + pwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, cmd); + if (pwdfile_fd) + goto error; + + virCommandAddArg(cmd, "--key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc,kdf=pbkdf2", + pwdfile_fd); + virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + pwdfile_fd = -1; + } + return cmd; error: diff --git a/src/util/virtpm.c b/src/util/virtpm.c index 692033e899..eda16a8c57 100644 --- a/src/util/virtpm.c +++ b/src/util/virtpm.c @@ -317,3 +317,19 @@ virTPMEmulatorInit(void) return 0; } + +bool +virTPMSwtpmCapsGet(unsigned int cap) +{ + if (virTPMEmulatorInit() < 0) + return false; + return virBitmapIsBitSet(swtpm_caps, cap); +} + +bool +virTPMSwtpmSetupCapsGet(unsigned int cap) +{ + if (virTPMEmulatorInit() < 0) + return false; + return virBitmapIsBitSet(swtpm_setup_caps, cap); +} diff --git a/src/util/virtpm.h b/src/util/virtpm.h index f3dc40cd68..40bde4c666 100644 --- a/src/util/virtpm.h +++ b/src/util/virtpm.h @@ -27,6 +27,9 @@ const char *virTPMGetSwtpmSetup(void); const char *virTPMGetSwtpmIoctl(void); int virTPMEmulatorInit(void); +bool virTPMSwtpmCapsGet(unsigned int cap); +bool virTPMSwtpmSetupCapsGet(unsigned int cap); + typedef enum { VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD, -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:50PM -0400, Stefan Berger wrote:
Allow vTPM state encryption when swtpm_setup and swtpm support passing a passphrase using a file descriptor.
This patch enables the encryption of the vTPM state only. It does not encrypt the state during migration, so the destination secret does not need to have the same password at this point.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 110 ++++++++++++++++++++++++++++++++++++++- src/util/virtpm.c | 16 ++++++ src/util/virtpm.h | 3 ++ 4 files changed, 129 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
This patch now passes the passphrase as a migration key to swtpm. This now encrypts the state of the TPM while a VM is migrated between hosts or when suspended into a file. Since the migration key secret is the same as the state encryption secret, this now requires that the migration destination host has the same secret value. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 3bec68bce6..75c1280aea 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -578,6 +578,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, bool created = false; char *pidfile; VIR_AUTOCLOSE pwdfile_fd = -1; + VIR_AUTOCLOSE migpwdfile_fd = -1; const unsigned char *secretuuid = NULL; if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, @@ -644,6 +645,9 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, pwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, cmd); if (pwdfile_fd) + migpwdfile_fd = qemuTPMSetupEncryption(tpm->data.emulator.secretuuid, + cmd); + if (pwdfile_fd < 0 || migpwdfile_fd < 0) goto error; virCommandAddArg(cmd, "--key"); @@ -651,6 +655,12 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, pwdfile_fd); virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); pwdfile_fd = -1; + + virCommandAddArg(cmd, "--migration-key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc,kdf=pbkdf2", + migpwdfile_fd); + virCommandPassFD(cmd, migpwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + migpwdfile_fd = -1; } return cmd; -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:51PM -0400, Stefan Berger wrote:
This patch now passes the passphrase as a migration key to swtpm. This now encrypts the state of the TPM while a VM is migrated between hosts or when suspended into a file. Since the migration key secret is the same as the state encryption secret, this now requires that the migration destination host has the same secret value.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
@@ -651,6 +655,12 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, pwdfile_fd); virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); pwdfile_fd = -1; + + virCommandAddArg(cmd, "--migration-key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc,kdf=pbkdf2", + migpwdfile_fd);
Why do we have to specify a kdf for migration, but not for the original on disk encryoptiuon ?
+ virCommandPassFD(cmd, migpwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + migpwdfile_fd = -1;
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/25/19 5:58 AM, Daniel P. Berrangé wrote:
On Fri, Jul 12, 2019 at 12:23:51PM -0400, Stefan Berger wrote:
This patch now passes the passphrase as a migration key to swtpm. This now encrypts the state of the TPM while a VM is migrated between hosts or when suspended into a file. Since the migration key secret is the same as the state encryption secret, this now requires that the migration destination host has the same secret value.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/qemu/qemu_tpm.c | 10 ++++++++++ 1 file changed, 10 insertions(+)
@@ -651,6 +655,12 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, pwdfile_fd); virCommandPassFD(cmd, pwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); pwdfile_fd = -1; + + virCommandAddArg(cmd, "--migration-key"); + virCommandAddArgFormat(cmd, "pwdfd=%d,mode=aes-256-cbc,kdf=pbkdf2", + migpwdfile_fd); Why do we have to specify a kdf for migration, but not for the original on disk encryoptiuon ?
swtpm_setup doesn't have it, but it's the default. Let me remove it.
+ virCommandPassFD(cmd, migpwdfile_fd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); + migpwdfile_fd = -1; Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Regards, Daniel
Since swtpm does not support getting started without password once it was created with encryption enabled, we don't allow encryption to be removed. Similarly, we do not allow encryption to be added once swtpm has run. We also prevent chaning the type of the TPM backend since the encrypted state is still around and the next time one was to switch back to the emulator backend and forgot the encryption the TPM would not work. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 56 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 28 ++++++++++++++++++++ src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_extdevice.h | 3 +++ 6 files changed, 93 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 7b0de160a6..88445dcde4 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -31426,3 +31426,59 @@ virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics) return true; } + + +static int +virDomainCheckTPMChanges(virDomainDefPtr def, + virDomainDefPtr newDef) +{ + bool oldEnc, newEnc; + + if (!def->tpm) + return 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + if (virFileExists(def->tpm->data.emulator.storagepath)) { + /* VM has been started */ + /* Once a VM was started with an encrypted state we allow + * less configuration changes. + */ + oldEnc = def->tpm->data.emulator.hassecretuuid; + if (oldEnc && def->tpm->type != newDef->tpm->type) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Changing the type of TPM is not allowed")); + return -1; + } + if (oldEnc && !newDef->tpm) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Removing an encrypted TPM is not allowed")); + return -1; + } + newEnc = newDef->tpm->data.emulator.hassecretuuid; + if (oldEnc != newEnc) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("TPM state encryption cannot be changed " + "once VM was started")); + return -1; + } + } + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return 0; +} + + +int +virDomainCheckDeviceChanges(virDomainDefPtr def, + virDomainDefPtr newDef) +{ + if (!def || !newDef) + return 0; + + return virDomainCheckTPMChanges(def, newDef); +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 3facf506ce..7482401983 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3624,3 +3624,7 @@ virDomainGraphicsGetRenderNode(const virDomainGraphicsDef *graphics); bool virDomainGraphicsNeedsAutoRenderNode(const virDomainGraphicsDef *graphics); + +int +virDomainCheckDeviceChanges(virDomainDefPtr def, virDomainDefPtr newDef) + ATTRIBUTE_NONNULL(2); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index b531ab1efa..ec3b893a2e 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -205,6 +205,7 @@ virDomainBootTypeFromString; virDomainBootTypeToString; virDomainCapabilitiesPolicyTypeToString; virDomainCapsFeatureTypeToString; +virDomainCheckDeviceChanges; virDomainChrConsoleTargetTypeFromString; virDomainChrConsoleTargetTypeToString; virDomainChrDefForeach; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ef2e980216..8f224582b6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -52,6 +52,7 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "virerror.h" #include "virlog.h" @@ -7568,6 +7569,30 @@ qemuDomainCreate(virDomainPtr dom) return qemuDomainCreateWithFlags(dom, 0); } +static int +qemuDomainCheckDeviceChanges(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + virDomainObjPtr vm; + int ret; + + vm = virDomainObjListFindByUUID(driver->domains, def->uuid); + if (!vm) + return 0; + + if (qemuExtDevicesInitPaths(driver, vm->def) < 0) { + ret = -1; + goto cleanup; + } + + ret = virDomainCheckDeviceChanges(vm->def, def); + + cleanup: + virDomainObjEndAPI(&vm); + + return ret; +} + static virDomainPtr qemuDomainDefineXMLFlags(virConnectPtr conn, const char *xml, @@ -7604,6 +7629,9 @@ qemuDomainDefineXMLFlags(virConnectPtr conn, if (virDomainDefineXMLFlagsEnsureACL(conn, def) < 0) goto cleanup; + if (qemuDomainCheckDeviceChanges(driver, def) < 0) + goto cleanup; + if (!(vm = virDomainObjListAdd(driver->domains, def, driver->xmlopt, 0, &oldDef))) diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index a21caefaba..e576bca165 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -79,7 +79,7 @@ qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, * stored and we can remove directories and files in case of domain XML * changes. */ -static int +int qemuExtDevicesInitPaths(virQEMUDriverPtr driver, virDomainDefPtr def) { diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index a72e05ce63..bbdb9a1cc2 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -53,3 +53,6 @@ bool qemuExtDevicesHasDevice(virDomainDefPtr def); int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, virDomainDefPtr def, virCgroupPtr cgroup); + +int qemuExtDevicesInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def); -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:52PM -0400, Stefan Berger wrote:
Since swtpm does not support getting started without password once it was created with encryption enabled, we don't allow encryption to be removed. Similarly, we do not allow encryption to be added once swtpm has run. We also prevent chaning the type of the TPM backend since the encrypted state is still around and the next time one was to switch back to the emulator backend and forgot the encryption the TPM would not work.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/conf/domain_conf.c | 56 +++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 +++ src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 28 ++++++++++++++++++++ src/qemu/qemu_extdevice.c | 2 +- src/qemu/qemu_extdevice.h | 3 +++ 6 files changed, 93 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Extend the Secret XML documentation with vtpm usage type. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatsecret.html.in | 61 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-) diff --git a/docs/formatsecret.html.in b/docs/formatsecret.html.in index defbe71731..8d0630a7c3 100644 --- a/docs/formatsecret.html.in +++ b/docs/formatsecret.html.in @@ -42,8 +42,8 @@ Specifies what this secret is used for. A mandatory <code>type</code> attribute specifies the usage category, currently only <code>volume</code>, <code>ceph</code>, <code>iscsi</code>, - and <code>tls</code> are defined. Specific usage categories - are described below. + <code>tls</code>, and <code>vtpm</code> are defined. Specific usage + categories are described below. </dd> </dl> @@ -322,6 +322,63 @@ Secret 718c71bd-67b5-4a2b-87ec-a24e8ca200dc created <pre> # MYSECRET=`printf %s "letmein" | base64` # virsh secret-set-value 718c71bd-67b5-4a2b-87ec-a24e8ca200dc $MYSECRET +Secret value set + + </pre> + + <h3><a id="vTPMUsageType">Usage type "vtpm"</a></h3> + + <p> + This secret is associated with a virtualized TPM (vTPM) and serves + as a passphrase for deriving a key from for encrypting the state + of the vTPM. + The <code><usage type='vtpm'></code> element must contain + a single <code>name</code> element that specifies a usage name + for the secret. The vTPM secret can then be used by UUID or by + this usage name via the <code><encryption></code> element of + a <a href="formatdomain.html#elementsTpm">tpm</a> when using an + emulator. + <span class="since">Since 5.6.0</span>. The following is an example + of the steps to be taken. First create a vtpm-secret.xml file: </p> + + <pre> +# cat vtpm-secret.xml +<secret ephemeral='no' private='yes'> + <description>sample vTPM secret</description> + <usage type='vtpm'> + <name>VTPM_example</name> + </usage> +</secret> + +# virsh secret-define vtpm-secret.xml +Secret 6dd3e4a5-1d76-44ce-961f-f119f5aad935 created + +# virsh secret-list + UUID Usage +---------------------------------------------------------------------------------------- + 6dd3e4a5-1d76-44ce-961f-f119f5aad935 vtpm VTPM_example + +# + + </pre> + + <p> + A secret may also be defined via the + <a href="html/libvirt-libvirt-secret.html#virSecretDefineXML"> + <code>virSecretDefineXML</code></a> API. + + Once the secret is defined, a secret value will need to be set. The + secret would be the passphrase used to decrypt the vTPM state. + The following is a simple example of using + <code>virsh secret-set-value</code> to set the secret value. The + <a href="html/libvirt-libvirt-secret.html#virSecretSetValue"> + <code>virSecretSetValue</code></a> API may also be used to set + a more secure secret without using printable/readable characters. + </p> + + <pre> +# MYSECRET=`printf %s "open sesame" | base64` +# virsh secret-set-value 6dd3e4a5-1d76-44ce-961f-f119f5aad935 $MYSECRET Secret value set </pre> -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:53PM -0400, Stefan Berger wrote:
Extend the Secret XML documentation with vtpm usage type.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> --- docs/formatsecret.html.in | 61 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 59 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Describe the encryption element in the TPM's domain XML. Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.html.in | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index a7a6ec32a5..1e27a3220b 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -8212,6 +8212,9 @@ qemu-kvm -net nic,model=? /dev/null TPM functionality for each VM. QEMU talks to it over a Unix socket. With the emulator device type each guest gets its own private TPM. <span class="since">'emulator' since 4.5.0</span> + The state of the TPM emulator can be encrypted by providing an + <code>encryption</code> element. + <span class="since">'encryption' since 5.6.0</span> </p> <p> Example: usage of the TPM Emulator @@ -8221,6 +8224,7 @@ qemu-kvm -net nic,model=? /dev/null <devices> <tpm model='tpm-tis'> <backend type='emulator' version='2.0'> + <encryption secret='6dd3e4a5-1d76-44ce-961f-f119f5aad935'/> </backend> </tpm> </devices> @@ -8283,6 +8287,14 @@ qemu-kvm -net nic,model=? /dev/null <li>'2.0' : creates a TPM 2.0</li> </ul> </dd> + <dt><code>encryption</code></dt> + <dd> + <p> + The <code>encryption</code> element allows the state of a TPM emulator + to be encrypted. The <code>secret</code> must reference a secret object + that holds the passphrase from which the encryption key will be derived. + </p> + </dd> </dl> <h4><a id="elementsNVRAM">NVRAM device</a></h4> -- 2.20.1
On Fri, Jul 12, 2019 at 12:23:54PM -0400, Stefan Berger wrote:
Describe the encryption element in the TPM's domain XML.
Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- docs/formatdomain.html.in | 12 ++++++++++++ 1 file changed, 12 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On 7/12/19 12:23 PM, Stefan Berger wrote:
This series of patches addresses the RFE in BZ 172830: https://bugzilla.redhat.com/show_bug.cgi?id=1728030
This series of patches adds support for vTPM state encryption by passing the read-end of a pipe's file descriptor to 'swtpm_setup' and 'swtpm' where they can read a passphrase from and derive a key from that passphrase.
The TPM's domain XML looks to enable state encryption looks like this:
<tpm model='tpm-tis'> <backend type='emulator' version='1.2'> <encryption secret='2c9ceaba-c6ef-4f38-86fd-6e3adb2df5cd'/> </backend> </tpm>
Hi Daniel, I adapted this now to what you suggested. Can you have a look ? Stefan
participants (3)
-
Daniel P. Berrangé -
Stefan Berger -
Stefan Berger