Hi On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

Add support for usage type vTPM to secret.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- include/libvirt/libvirt-secret.h | 1 + src/conf/secret_conf.c | 13 +++++++++++++ src/util/virsecret.c | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-)

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 -- 2.20.1

Looks good, but docs/schemas/secret.rng change should probably be part of this patch. other than that, Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

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> --- docs/schemas/secret.rng | 10 ++++++++++ tests/secretxml2xmlin/usage-vtpm.xml | 7 +++++++ tests/secretxml2xmltest.c | 1 + 3 files changed, 18 insertions(+) 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>

should be in previous patch

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

other than that, Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

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>

--- 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

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

Add VIR_STORAGE_ENCRYPTION_FORMAT_VTPM with string 'vtpm' for support of encrypting vTPM storage.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- src/qemu/qemu_block.c | 1 + src/util/virstorageencryption.c | 2 +- src/util/virstorageencryption.h | 1 + 3 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c index 0a6522577d..c3296c36f5 100644 --- a/src/qemu/qemu_block.c +++ b/src/qemu/qemu_block.c @@ -1184,6 +1184,7 @@ qemuBlockStorageSourceGetCryptoProps(virStorageSourcePtr src, encformat = "luks"; break;

+ case VIR_STORAGE_ENCRYPTION_FORMAT_VTPM: case VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT: case VIR_STORAGE_ENCRYPTION_FORMAT_LAST: default: diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 49df7fddd8..4c7356d47a 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -44,7 +44,7 @@ VIR_ENUM_IMPL(virStorageEncryptionSecret,

VIR_ENUM_IMPL(virStorageEncryptionFormat, VIR_STORAGE_ENCRYPTION_FORMAT_LAST, - "default", "qcow", "luks", + "default", "qcow", "luks", "vtpm", );

static void diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h index 3e5485d88b..29bd00056c 100644 --- a/src/util/virstorageencryption.h +++ b/src/util/virstorageencryption.h @@ -59,6 +59,7 @@ typedef enum { VIR_STORAGE_ENCRYPTION_FORMAT_DEFAULT = 0, VIR_STORAGE_ENCRYPTION_FORMAT_QCOW, /* Both qcow and qcow2 */ VIR_STORAGE_ENCRYPTION_FORMAT_LUKS, + VIR_STORAGE_ENCRYPTION_FORMAT_VTPM,

VIR_STORAGE_ENCRYPTION_FORMAT_LAST, } virStorageEncryptionFormatType; -- 2.20.1

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

Extend the TPM device XML parser and XML generator with emulator state encryption support.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- src/conf/domain_conf.c | 40 +++++++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 1 + 2 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3323c9a5b1..df6238c299 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -2950,6 +2950,7 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def) virDomainChrSourceDefClear(&def->data.emulator.source); VIR_FREE(def->data.emulator.storagepath); VIR_FREE(def->data.emulator.logfile); + virStorageEncryptionFree(def->data.emulator.encryption); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; @@ -13048,6 +13049,16 @@ 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 format='vtpm'> + * <secret type='passphrase' uuid='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + * </encryption> + * </backend> + * </tpm> */ static virDomainTPMDefPtr virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, @@ -13063,6 +13074,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_AUTOFREE(char *) backend = NULL; VIR_AUTOFREE(char *) version = NULL; VIR_AUTOFREE(xmlNodePtr *) backends = NULL; + xmlNodePtr encnode = NULL;

if (VIR_ALLOC(def) < 0) return NULL; @@ -13126,6 +13138,21 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV; break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + encnode = virXPathNode("./backend/encryption", ctxt); + if (encnode) { + def->data.emulator.encryption = + virStorageEncryptionParseNode(encnode, ctxt); + if (!def->data.emulator.encryption) + goto error; + if (def->data.emulator.encryption->format != + VIR_STORAGE_ENCRYPTION_FORMAT_VTPM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported vTPM encryption type '%s'"), + virStorageEncryptionFormatTypeToString( + def->data.emulator.encryption->format)); + goto error; + } + } break; case VIR_DOMAIN_TPM_TYPE_LAST: goto error; @@ -25949,8 +25976,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.encryption) { + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); + if (virStorageEncryptionFormat(buf, + def->data.emulator.encryption) < 0) + return -1; + 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..a03986623a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1270,6 +1270,7 @@ struct _virDomainTPMDef { virDomainChrSourceDef source; char *storagepath; char *logfile; + virStorageEncryptionPtr encryption; } emulator; } data; }; -- 2.20.1

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

Extend the TPM XML schema with support for an encryption node.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- docs/schemas/domaincommon.rng | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+)

diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 31db599ab9..4a4f4f8a4d 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,35 @@ </optional> </define>

+ <define name="tpm-backend-emulator-encryption"> + <optional> + <element name="encryption"> + <attribute name="format"> + <value>vtpm</value> + </attribute> + <ref name="vtpmAuthSecret"/> + </element> + </optional> + </define> + + <define name='vtpmAuthSecret'> + <element name='secret'> + <attribute name='type'> + <choice> + <value>passphrase</value> + </choice> + </attribute> + <choice> + <attribute name='uuid'> + <ref name="UUID"/> + </attribute> + <attribute name='usage'> + <text/> + </attribute> + </choice> + </element> + </define> + <define name="vsock"> <element name="vsock"> <optional> -- 2.20.1

On Tue, Jul 9, 2019 at 9:25 PM Stefan Berger <stefanb@linux.vnet.ibm.com> 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 | 34 +++++++++++++++++ .../tpm-emulator-tpm2-enc.xml | 38 +++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 3 files changed, 73 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..3838518e65 --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2-enc.xml @@ -0,0 +1,34 @@ +<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 format='vtpm'> + <secret type='passphrase' uuid='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + </encryption> + </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..7083fc9c13 --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2-enc.xml @@ -0,0 +1,38 @@ +<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 format='vtpm'> + <secret type='passphrase' uuid='32ee7e76-2178-47a1-ab7b-269e6e348015'/> + </encryption> + </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 Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> 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

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 Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

Move virtpm.c from utils dir to its own tpm dir. This change is mostly driven by the later introduction of virtpm_conf.c where the define function like XYZTypeFromString() that we cannot include from utils dir.

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- po/POTFILES | 2 +- src/Makefile.am | 1 + src/conf/Makefile.inc.am | 1 + src/libvirt_private.syms | 8 ++++---- src/qemu/Makefile.inc.am | 1 + src/security/Makefile.inc.am | 1 + src/tpm/Makefile.inc.am | 17 +++++++++++++++++ src/{util => tpm}/virtpm.c | 0 src/{util => tpm}/virtpm.h | 0 src/util/Makefile.inc.am | 2 -- tests/Makefile.am | 1 + 11 files changed, 27 insertions(+), 7 deletions(-) create mode 100644 src/tpm/Makefile.inc.am rename src/{util => tpm}/virtpm.c (100%) rename src/{util => tpm}/virtpm.h (100%)

diff --git a/po/POTFILES b/po/POTFILES index 8017712ff4..e3c1a8edae 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -188,6 +188,7 @@ src/storage/storage_backend_zfs.c src/storage/storage_driver.c src/storage/storage_util.c src/test/test_driver.c +src/tpm/virtpm.c src/util/iohelper.c src/util/viralloc.c src/util/virarptable.c @@ -266,7 +267,6 @@ src/util/virsysinfo.c src/util/virthreadjob.c src/util/virthreadpool.c src/util/virtime.c -src/util/virtpm.c src/util/virtypedparam.c src/util/viruri.c src/util/virusb.c diff --git a/src/Makefile.am b/src/Makefile.am index 0b562dc250..5f7c9001fe 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -122,6 +122,7 @@ include node_device/Makefile.inc.am include secret/Makefile.inc.am include storage/Makefile.inc.am include remote/Makefile.inc.am +include tpm/Makefile.inc.am

THREAD_LIBS = $(LIB_PTHREAD) $(LTLIBMULTITHREAD) diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 6b52ba674b..08c7c9da7f 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -178,6 +178,7 @@ libvirt_la_BUILT_LIBADD += libvirt_conf.la libvirt_conf_la_SOURCES = $(CONF_SOURCES) libvirt_conf_la_CFLAGS = \ -I$(srcdir)/conf \ + -I$(srcdir)/tpm \ $(AM_CFLAGS) \ $(NULL) libvirt_conf_la_LDFLAGS = $(AM_LDFLAGS) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 34937adc5d..e29007cab1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1445,6 +1445,10 @@ virSecurityManagerTransactionStart; virSecurityManagerVerify;

+# tpm/virtpm.h +virTPMCreateCancelPath; + + # util/viralloc.h virAlloc; virAllocN; @@ -3175,10 +3179,6 @@ virTimeStringThen; virTimeStringThenRaw;

-# util/virtpm.h -virTPMCreateCancelPath; - - # util/virtypedparam.h virTypedParameterAssign; virTypedParameterAssignFromStr; diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 254ba07dc0..72b28e731d 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -84,6 +84,7 @@ libvirt_driver_qemu_impl_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ -I$(srcdir)/secret \ + -I$(srcdir)/tpm \ $(AM_CFLAGS) \ $(NULL) libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) diff --git a/src/security/Makefile.inc.am b/src/security/Makefile.inc.am index 64e0f46857..3e110596dc 100644 --- a/src/security/Makefile.inc.am +++ b/src/security/Makefile.inc.am @@ -47,6 +47,7 @@ noinst_LTLIBRARIES += libvirt_security_manager.la libvirt_la_BUILT_LIBADD += libvirt_security_manager.la libvirt_security_manager_la_CFLAGS = \ -I$(srcdir)/conf \ + -I$(srcdir)/tpm \ $(AM_CFLAGS) \ $(NULL) libvirt_security_manager_la_LDFLAGS = $(AM_LDFLAGS) diff --git a/src/tpm/Makefile.inc.am b/src/tpm/Makefile.inc.am new file mode 100644 index 0000000000..1f5131bf34 --- /dev/null +++ b/src/tpm/Makefile.inc.am @@ -0,0 +1,17 @@ +# vim: filetype=automake + +TPM_UTIL_SOURCES = \ + tpm/virtpm.h \ + tpm/virtpm.c \ + $(NULL) + + +EXTRA_DIST += \ + $(TPM_UTIL_SOURCES) \ + $(NULL) + +noinst_LTLIBRARIES += libvirt_tpm.la +libvirt_la_BUILT_LIBADD += libvirt_tpm.la +libvirt_tpm_la_CFLAGS = $(AM_CFLAGS) +libvirt_tpm_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_tpm_la_SOURCES = $(TPM_UTIL_SOURCES) diff --git a/src/util/virtpm.c b/src/tpm/virtpm.c similarity index 100% rename from src/util/virtpm.c rename to src/tpm/virtpm.c diff --git a/src/util/virtpm.h b/src/tpm/virtpm.h similarity index 100% rename from src/util/virtpm.h rename to src/tpm/virtpm.h diff --git a/src/util/Makefile.inc.am b/src/util/Makefile.inc.am index c757f5a6ae..8b2ff6be6b 100644 --- a/src/util/Makefile.inc.am +++ b/src/util/Makefile.inc.am @@ -205,8 +205,6 @@ UTIL_SOURCES = \ util/virthreadpool.h \ util/virtime.c \ util/virtime.h \ - util/virtpm.c \ - util/virtpm.h \ util/virtypedparam.c \ util/virtypedparam.h \ util/virusb.c \ diff --git a/tests/Makefile.am b/tests/Makefile.am index 115afa1c1a..ffc9f7f92e 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -25,6 +25,7 @@ AM_CPPFLAGS = \ -I$(top_builddir)/src -I$(top_srcdir)/src \ -I$(top_srcdir)/src/util \ -I$(top_srcdir)/src/conf \ + -I$(top_srcdir)/src/tpm \ $(NULL)

WARN_CFLAGS += $(RELAXED_FRAME_LIMIT_CFLAGS) -- 2.20.1

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> 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>

Couldn't there be a TOCTOU issue? Anyway, for the move: Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

--- src/libvirt_private.syms | 4 ++ src/qemu/qemu_tpm.c | 83 ++++-------------------------------- src/tpm/virtpm.c | 91 ++++++++++++++++++++++++++++++++++++++++ src/tpm/virtpm.h | 5 +++ 4 files changed, 108 insertions(+), 75 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e29007cab1..e33d7d9f14 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1447,6 +1447,10 @@ virSecurityManagerVerify;

# tpm/virtpm.h virTPMCreateCancelPath; +virTPMEmulatorInit; +virTPMGetSwtpm; +virTPMGetSwtpmIoctl; +virTPMGetSwtpmSetup;

# util/viralloc.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/tpm/virtpm.c b/src/tpm/virtpm.c index 583b9a64a4..4635d8add0 100644 --- a/src/tpm/virtpm.c +++ b/src/tpm/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/tpm/virtpm.h b/src/tpm/virtpm.h index 4408bdb217..66d55fb231 100644 --- a/src/tpm/virtpm.h +++ b/src/tpm/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 7/9/19 4:24 PM, Marc-André Lureau wrote:

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> 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/conf/Makefile.inc.am | 6 ++ src/conf/virtpm_conf.c | 36 ++++++++++++ src/conf/virtpm_conf.h | 36 ++++++++++++ src/libvirt_private.syms | 5 ++ src/tpm/Makefile.inc.am | 5 +- src/tpm/virtpm.c | 123 ++++++++++++++++++++++++++++++++++++++- 6 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 src/conf/virtpm_conf.c create mode 100644 src/conf/virtpm_conf.h

diff --git a/src/conf/Makefile.inc.am b/src/conf/Makefile.inc.am index 08c7c9da7f..e42425fcc5 100644 --- a/src/conf/Makefile.inc.am +++ b/src/conf/Makefile.inc.am @@ -153,6 +153,11 @@ DEVICE_CONF_SOURCES = \ conf/device_conf.h \ $(NULL)

+TPM_CONF_SOURCES = \ + conf/virtpm_conf.c \ + conf/virtpm_conf.h \ + $(NULL) + CONF_SOURCES = \ $(NETDEV_CONF_SOURCES) \ $(DOMAIN_CONF_SOURCES) \ @@ -171,6 +176,7 @@ CONF_SOURCES = \ $(CPU_CONF_SOURCES) \ $(CHRDEV_CONF_SOURCES) \ $(DEVICE_CONF_SOURCES) \ + $(TPM_CONF_SOURCES) \ $(NULL)

noinst_LTLIBRARIES += libvirt_conf.la diff --git a/src/conf/virtpm_conf.c b/src/conf/virtpm_conf.c new file mode 100644 index 0000000000..12e69e67b3 --- /dev/null +++ b/src/conf/virtpm_conf.c @@ -0,0 +1,36 @@ +/* + * virtpm_conf.c: vTPM XML processing + * + * Copyright (C) 2019 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#include <config.h> + +#include "virenum.h" +#include "virtpm_conf.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", +); diff --git a/src/conf/virtpm_conf.h b/src/conf/virtpm_conf.h new file mode 100644 index 0000000000..73c6c67271 --- /dev/null +++ b/src/conf/virtpm_conf.h @@ -0,0 +1,36 @@ +/* + * virtpm_conf.h: vTPM XML processing + * + * Copyright (C) 2019 IBM Corporation + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library. If not, see + * <http://www.gnu.org/licenses/>. + */ + +#pragma once + +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); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index e33d7d9f14..d2045895a1 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1236,6 +1236,11 @@ virStoragePoolObjVolumeGetNames; virStoragePoolObjVolumeListExport;

+# conf/virtpm_conf.h +virTPMSwtpmFeatureTypeFromString; +virTPMSwtpmSetupFeatureTypeFromString; + + # cpu/cpu.h cpuDecode; cpuEncode; diff --git a/src/tpm/Makefile.inc.am b/src/tpm/Makefile.inc.am index 1f5131bf34..d8a15c406c 100644 --- a/src/tpm/Makefile.inc.am +++ b/src/tpm/Makefile.inc.am @@ -12,6 +12,9 @@ EXTRA_DIST += \

noinst_LTLIBRARIES += libvirt_tpm.la libvirt_la_BUILT_LIBADD += libvirt_tpm.la -libvirt_tpm_la_CFLAGS = $(AM_CFLAGS) +libvirt_tpm_la_CFLAGS = \ + -I$(srcdir)/conf \ + $(AM_CFLAGS) \ + $(NULL) libvirt_tpm_la_LDFLAGS = $(AM_LDFLAGS) libvirt_tpm_la_SOURCES = $(TPM_UTIL_SOURCES) diff --git a/src/tpm/virtpm.c b/src/tpm/virtpm.c index e4735f9c4d..42dd2b1bb2 100644 --- a/src/tpm/virtpm.c +++ b/src/tpm/virtpm.c @@ -27,6 +27,10 @@ #include "viralloc.h" #include "virfile.h" #include "virtpm.h" +#include "vircommand.h" +#include "virbitmap.h" +#include "virjson.h" +#include "virtpm_conf.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -74,17 +78,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 +118,106 @@ 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; + char *outbuf = NULL; + virJSONValuePtr json = NULL; + virJSONValuePtr featureList; + virJSONValuePtr item; + size_t idx; + const char *str; + int typ; + + if (!(bitmap = virBitmapNewEmpty())) + return NULL; + + virCommandSetOutputBuffer(cmd, &outbuf); + /* We allow the command to fail since older versions of it may + * not support --print-capabilities + */ + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) + goto cleanup; We should probably fail if the command failed to run, and skip if exitstatus != 0.

...

+ + 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: + VIR_FREE(outbuf); + virJSONValueFree(json); Please use VIR_AUTOFREE and VIR_AUTOPTR

The new stuff. Forgot about it... Will fix.

...

+ + 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) +{ + virCommandPtr cmd; + virBitmapPtr bitmap; + + cmd = virCommandNew(exec); + if (!cmd) + return NULL; + if (param1) + virCommandAddArg(cmd, param1); Hmm, I would expect --print-capabilites to be top-level argument, not a sub-command argument.

'swtpm' has 'swtpm socket' and 'swtpm cuse' and advertises different capabilities for 'swtpm socket  --print-capabilities' than for 'swtpm cuse --print-capabilities'. It doesn't support 'swtpm --print-capabilities'. Thanks,    Stefan

On Tue, Jul 9, 2019 at 9:24 PM Stefan Berger <stefanb@linux.vnet.ibm.com> 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.

You could add that it is addressed in the following patch

Signed-off-by: Stefan Berger <stefanb@linux.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 101 ++++++++++++++++++++++++++++++++++++++- src/tpm/virtpm.c | 16 +++++++ src/tpm/virtpm.h | 3 ++ 4 files changed, 120 insertions(+), 2 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index d2045895a1..d693f7facb 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1456,6 +1456,8 @@ virTPMEmulatorInit; virTPMGetSwtpm; virTPMGetSwtpmIoctl; virTPMGetSwtpmSetup; +virTPMSwtpmCapsGet; +virTPMSwtpmSetupCapsGet;

# util/viralloc.h diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 2afa8db448..6e7d38b7e0 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -43,6 +43,8 @@ #include "dirname.h" #include "qemu_tpm.h" #include "virtpm.h" +#include "secret_util.h" +#include "virtpm_conf.h"

#define VIR_FROM_THIS VIR_FROM_NONE

@@ -372,6 +374,60 @@ qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, return ret; }

+/* + * qemuTPMSetupEncryption + * + * @encryption: pointer to virStorageEncryption holding secret + * + * 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(virStorageEncryptionPtr encryption) +{ + int ret = -1; + int pipefd[2] = { -1, -1 }; + virConnectPtr conn; + uint8_t *secret = NULL; + size_t secret_len; + + conn = virGetConnectSecret(); + if (!conn) + return -1; + + if (virSecretGetSecretString(conn, &encryption->secrets[0]->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 (safewrite(pipefd[1], secret, secret_len) != secret_len) + goto error;

Hmm, I am not sure you can reliably buffer data on a pipe end and close it before the other end read. Got any documentation pointer about that?

+ + ret = pipefd[0]; + + cleanup: + VIR_FREE(secret); + VIR_FORCE_CLOSE(pipefd[1]); + virObjectUnref(conn); + + return ret; + + error: + VIR_FORCE_CLOSE(pipefd[0]); + + goto cleanup; +}

/* * qemuTPMEmulatorRunSetup @@ -386,6 +442,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 +455,15 @@ qemuTPMEmulatorRunSetup(const char *storagepath, uid_t swtpm_user, gid_t swtpm_group, const char *logfile, - const virDomainTPMVersion tpmversion) + const virDomainTPMVersion tpmversion, + virStorageEncryptionPtr encryption) { virCommandPtr cmd = NULL; int exitstatus; int ret = -1; char uuid[VIR_UUID_STRING_BUFLEN]; char *vmid = NULL; + int pwdfile_fd = -1;

if (!privileged && tpmversion == VIR_DOMAIN_TPM_VERSION_1_2) return virFileWriteStr(logfile, @@ -434,6 +493,22 @@ qemuTPMEmulatorRunSetup(const char *storagepath, break; }

+ if (encryption) { + 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(encryption)) < 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); + }

virCommandAddArgList(cmd, "--tpm-state", storagepath, @@ -461,6 +536,7 @@ qemuTPMEmulatorRunSetup(const char *storagepath, cleanup: VIR_FREE(vmid); virCommandFree(cmd); + VIR_FORCE_CLOSE(pwdfile_fd);

virCommandPassFD() doc says: "The parent should cease using the @fd when this call completes"

return ret; } @@ -496,6 +572,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, virCommandPtr cmd = NULL; bool created = false; char *pidfile; + int pwdfile_fd = -1;

if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, &created, swtpm_user, swtpm_group) < 0) @@ -504,7 +581,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, 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, + tpm->data.emulator.encryption) < 0) goto error;

unlink(tpm->data.emulator.source.data.nix.path); @@ -547,11 +625,30 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, virCommandAddArgFormat(cmd, "file=%s", pidfile); VIR_FREE(pidfile);

+ if (tpm->data.emulator.encryption) { + 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.encryption); + if (pwdfile_fd < 0) + 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); + } + return cmd;

error: if (created) qemuTPMDeleteEmulatorStorage(tpm); + VIR_FORCE_CLOSE(pwdfile_fd);

same, and similarly in next patch

virCommandFree(cmd);

diff --git a/src/tpm/virtpm.c b/src/tpm/virtpm.c index 42dd2b1bb2..7e95dbad6f 100644 --- a/src/tpm/virtpm.c +++ b/src/tpm/virtpm.c @@ -312,3 +312,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/tpm/virtpm.h b/src/tpm/virtpm.h index 66d55fb231..a8bb6e1ba0 100644 --- a/src/tpm/virtpm.h +++ b/src/tpm/virtpm.h @@ -26,3 +26,6 @@ const char *virTPMGetSwtpm(void); const char *virTPMGetSwtpmSetup(void); const char *virTPMGetSwtpmIoctl(void); int virTPMEmulatorInit(void); + +bool virTPMSwtpmCapsGet(unsigned int cap); +bool virTPMSwtpmSetupCapsGet(unsigned int cap); -- 2.20.1