[libvirt] [PATCH v4 00/11] Add support for TPM emulator

This series of patches adds support for the TPM emulator backend that is available in QEMU and based on swtpm + libtpms. It allows to attach a TPM 1.2 or 2 to a QEMU VM. sVirt labels are used for labeling the swtpm process, its Unix socket, and log file with the same label that the QEMU process gets. Besides that swtpm is added to the emulator cgroup to restrict its CPU usage. The device XML can be changed from a TPM 1.2 to a TPM 2 and back to a TPM 1.2. The device state is not removed during those changes but only when the domain is undefined. The swtpm needs persistent storage to store its state. For that I am using the uuid of the VM as part of the path since the name of the VM can be changed. Logfiles, PID files, and socket names are based on the name of the VM, though. Stefan v3->v4: - Addressed John Ferlan's comments - Fixed bugs I found while testing - rebased on latest tip Stefan Berger (11): conf: Add support for external swtpm TPM emulator to domain XML qemu: Extend QEMU capabilities with 'tpm-emulator' util: Implement virFileChownFiles() security: Add DAC and SELinux security for tpm-emulator qemu: Extend qemu_conf with tpm-emulator support qemu: Extend QEMU with external TPM support qemu: Add support for external swtpm TPM emulator tests: Add test cases for external swtpm TPM emulator security: Label the external swtpm with SELinux labels tpm: Add support for choosing emulation of a TPM 2 qemu: Add swtpm to emulator cgroup docs/formatdomain.html.in | 43 + docs/schemas/domaincommon.rng | 17 + libvirt.spec.in | 2 + src/conf/domain_audit.c | 2 + src/conf/domain_conf.c | 53 +- src/conf/domain_conf.h | 13 + src/libvirt_private.syms | 3 + src/qemu/Makefile.inc.am | 10 + src/qemu/libvirtd_qemu.aug | 5 + src/qemu/qemu.conf | 8 + src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_cgroup.c | 36 + src/qemu/qemu_cgroup.h | 2 + src/qemu/qemu_command.c | 34 +- src/qemu/qemu_conf.c | 43 + src/qemu/qemu_conf.h | 6 + src/qemu/qemu_domain.c | 3 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 177 ++++ src/qemu/qemu_extdevice.h | 59 ++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 16 + src/qemu/qemu_tpm.c | 962 +++++++++++++++++++++ src/qemu/qemu_tpm.h | 56 ++ src/qemu/test_libvirtd_qemu.aug.in | 2 + src/security/security_dac.c | 7 + src/security/security_driver.h | 7 + src/security/security_manager.c | 36 + src/security/security_manager.h | 6 + src/security/security_selinux.c | 172 ++++ src/security/security_stack.c | 40 + src/util/virfile.c | 55 ++ src/util/virfile.h | 3 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + .../tpm-emulator-tpm2.x86_64-latest.args | 33 + tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 + .../tpm-emulator.x86_64-latest.args | 33 + tests/qemuxml2argvdata/tpm-emulator.xml | 30 + tests/qemuxml2argvtest.c | 16 +- tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 + tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 + tests/qemuxml2xmltest.c | 1 + 47 files changed, 2098 insertions(+), 10 deletions(-) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml create mode 100644 tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml -- 2.5.5

This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows: <tpm model='tpm-tis'> <backend type='emulator'/> </tpm> The XML will currently only define a TPM 1.2. Extend the documentation. Add a test case testing the XML parser and formatter. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 28 +++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++++ src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/security/security_dac.c | 2 ++ src/security/security_selinux.c | 2 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 30 +++++++++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..4f56784 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7650,6 +7650,26 @@ qemu-kvm -net nic,model=? /dev/null </devices> ... </pre> + + <p> + The emulator device type gives access to a TPM emulator providing + TPM functionlity 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.4.0</span> + </p> + <p> + Example: usage of the TPM Emulator + </p> +<pre> + ... + <devices> + <tpm model='tpm-tis'> + <backend type='emulator'> + </backend> + </tpm> + </devices> + ... +</pre> <dl> <dt><code>model</code></dt> <dd> @@ -7683,6 +7703,16 @@ qemu-kvm -net nic,model=? /dev/null </p> </dd> </dl> + <dl> + <dt><code>emulator</code></dt> + <dd> + <p> + For this backend type the 'swtpm' TPM Emulator must be installed on the + host. Libvirt will automatically start an independent TPM emulator + for each QEMU guest requesting access to it. + </p> + </dd> + </dl> </dd> </dl> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a9a1020 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4137,6 +4137,11 @@ </attribute> <ref name="tpm-passthrough-device"/> </group> + <group> + <attribute name="type"> + <value>emulator</value> + </attribute> + </group> </choice> </element> </define> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 82868bc..25cccdd 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -586,6 +586,8 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm, "virt=%s resrc=dev reason=%s %s uuid=%s %s", virt, reason, vmname, uuidstr, device); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + break; case VIR_DOMAIN_TPM_TYPE_LAST: default: break; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..21b66d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -864,7 +864,8 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, "tpm-crb") VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, - "passthrough") + "passthrough", + "emulator") VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, "intel") @@ -2601,6 +2602,11 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def) case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: VIR_FREE(def->data.passthrough.source.data.file.path); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + VIR_FREE(def->data.emulator.source.data.nix.path); + VIR_FREE(def->data.emulator.storagepath); + VIR_FREE(def->data.emulator.logfile); + break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -12585,6 +12591,11 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * </backend> * </tpm> * + * or like this: + * + * <tpm model='tpm-tis'> + * <backend type='emulator'/> + * </tpm> */ static virDomainTPMDefPtr virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, @@ -12651,6 +12662,8 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, def->data.passthrough.source.type = VIR_DOMAIN_CHR_TYPE_DEV; path = NULL; break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + break; case VIR_DOMAIN_TPM_TYPE_LAST: goto error; } @@ -24818,22 +24831,25 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<tpm model='%s'>\n", virDomainTPMModelTypeToString(def->model)); virBufferAdjustIndent(buf, 2); - virBufferAsprintf(buf, "<backend type='%s'>\n", + virBufferAsprintf(buf, "<backend type='%s'", virDomainTPMBackendTypeToString(def->type)); - virBufferAdjustIndent(buf, 2); switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + virBufferAddLit(buf, ">\n"); + virBufferAdjustIndent(buf, 2); virBufferEscapeString(buf, "<device path='%s'/>\n", def->data.passthrough.source.data.file.path); + virBufferAdjustIndent(buf, -2); + virBufferAddLit(buf, "</backend>\n"); + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + virBufferAddLit(buf, "/>\n"); break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } - virBufferAdjustIndent(buf, -2); - virBufferAddLit(buf, "</backend>\n"); - virDomainDeviceInfoFormat(buf, &def->info, flags); virBufferAdjustIndent(buf, -2); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..c304b08 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1286,6 +1286,7 @@ typedef enum { typedef enum { VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, + VIR_DOMAIN_TPM_TYPE_EMULATOR, VIR_DOMAIN_TPM_TYPE_LAST } virDomainTPMBackendType; @@ -1300,6 +1301,11 @@ struct _virDomainTPMDef { struct { virDomainChrSourceDef source; } passthrough; + struct { + virDomainChrSourceDef source; + char *storagepath; + char *logfile; + } emulator; } data; }; @@ -2814,6 +2820,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); void virDomainTPMDefFree(virDomainTPMDefPtr def); +void virDomainTPMDelete(virDomainDefPtr def); typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev, diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index d88eb78..1a5adca 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -278,6 +278,7 @@ qemuSetupTPMCgroup(virDomainObjPtr vm) case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: ret = qemuSetupChrSourceCgroup(vm, &dev->data.passthrough.source); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: break; } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 08f67a4..151f4fc 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9422,6 +9422,7 @@ qemuBuildTPMBackendStr(const virDomainDef *def, VIR_FREE(cancel_path); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: goto error; } diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 9bb6d8a..774a102 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -10365,6 +10365,7 @@ qemuDomainSetupTPM(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED, return -1; break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: /* nada */ break; diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 8938e2d..3ab2299 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1372,6 +1372,7 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr, &tpm->data.passthrough.source, false); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -1393,6 +1394,7 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr, &tpm->data.passthrough.source, false); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: break; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5f74ef7..5d20fda 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1472,6 +1472,7 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr, return -1; } break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -1505,6 +1506,7 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr, VIR_FREE(cancel_path); } break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: case VIR_DOMAIN_TPM_TYPE_LAST: break; } diff --git a/tests/qemuxml2argvdata/tpm-emulator.xml b/tests/qemuxml2argvdata/tpm-emulator.xml new file mode 100644 index 0000000..7f1e575 --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator.xml @@ -0,0 +1,30 @@ +<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'/> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/tpm-emulator.xml b/tests/qemuxml2xmloutdata/tpm-emulator.xml new file mode 100644 index 0000000..1b66e8b --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator.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'> + <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'/> + </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 53a26a0..9f8f08f 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -673,6 +673,7 @@ mymain(void) DO_TEST("disk-copy_on_read", NONE); DO_TEST("tpm-passthrough", NONE); DO_TEST("tpm-passthrough-crb", NONE); + DO_TEST("tpm-emulator", NONE); DO_TEST("metadata", NONE); DO_TEST("metadata-duplicate", NONE); -- 2.5.5

On 05/10/2018 05:57 PM, Stefan Berger wrote:
This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows:
<tpm model='tpm-tis'> <backend type='emulator'/> </tpm>
The XML will currently only define a TPM 1.2.
Extend the documentation.
Add a test case testing the XML parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 28 +++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++++ src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/security/security_dac.c | 2 ++ src/security/security_selinux.c | 2 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 30 +++++++++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
Even though R-by in place... [...]
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..c304b08 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1286,6 +1286,7 @@ typedef enum {
typedef enum { VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, + VIR_DOMAIN_TPM_TYPE_EMULATOR,
VIR_DOMAIN_TPM_TYPE_LAST } virDomainTPMBackendType; @@ -1300,6 +1301,11 @@ struct _virDomainTPMDef { struct { virDomainChrSourceDef source; } passthrough; + struct { + virDomainChrSourceDef source; + char *storagepath; + char *logfile; + } emulator; } data; };
@@ -2814,6 +2820,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); void virDomainTPMDefFree(virDomainTPMDefPtr def); +void virDomainTPMDelete(virDomainDefPtr def);
No longer an API - so this can be removed.
typedef int (*virDomainDeviceInfoCallback)(virDomainDefPtr def, virDomainDeviceDefPtr dev,
[...] John

On 05/15/2018 07:17 AM, John Ferlan wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows:
<tpm model='tpm-tis'> <backend type='emulator'/> </tpm>
The XML will currently only define a TPM 1.2.
Extend the documentation.
Add a test case testing the XML parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 28 +++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++++ src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/security/security_dac.c | 2 ++ src/security/security_selinux.c | 2 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 30 +++++++++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
Even though R-by in place...
[...]
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 15d228b..c304b08 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1286,6 +1286,7 @@ typedef enum {
typedef enum { VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, + VIR_DOMAIN_TPM_TYPE_EMULATOR,
VIR_DOMAIN_TPM_TYPE_LAST } virDomainTPMBackendType; @@ -1300,6 +1301,11 @@ struct _virDomainTPMDef { struct { virDomainChrSourceDef source; } passthrough; + struct { + virDomainChrSourceDef source; + char *storagepath; + char *logfile; + } emulator; } data; };
@@ -2814,6 +2820,7 @@ int virDomainDeviceAddressIsValid(virDomainDeviceInfoPtr info, int type); virDomainDeviceInfoPtr virDomainDeviceGetInfo(virDomainDeviceDefPtr device); void virDomainTPMDefFree(virDomainTPMDefPtr def); +void virDomainTPMDelete(virDomainDefPtr def); No longer an API - so this can be removed.
Fixed. Stefan

On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows:
<tpm model='tpm-tis'> <backend type='emulator'/> </tpm>
The XML will currently only define a TPM 1.2.
Extend the documentation.
Add a test case testing the XML parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 28 +++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++++ src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/security/security_dac.c | 2 ++ src/security/security_selinux.c | 2 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 30 +++++++++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..4f56784 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7650,6 +7650,26 @@ qemu-kvm -net nic,model=? /dev/null </devices> ... </pre> + + <p> + The emulator device type gives access to a TPM emulator providing + TPM functionlity 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.4.0</span> + </p> + <p> + Example: usage of the TPM Emulator + </p> +<pre> + ... + <devices> + <tpm model='tpm-tis'> + <backend type='emulator'> + </backend> + </tpm> + </devices> + ... +</pre> <dl> <dt><code>model</code></dt> <dd> @@ -7683,6 +7703,16 @@ qemu-kvm -net nic,model=? /dev/null </p> </dd> </dl> + <dl> + <dt><code>emulator</code></dt> + <dd> + <p> + For this backend type the 'swtpm' TPM Emulator must be installed on the + host. Libvirt will automatically start an independent TPM emulator + for each QEMU guest requesting access to it. + </p> + </dd> + </dl> </dd> </dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a9a1020 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4137,6 +4137,11 @@ </attribute> <ref name="tpm-passthrough-device"/> </group> + <group> + <attribute name="type"> + <value>emulator</value> + </attribute> + </group> </choice> </element> </define> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 82868bc..25cccdd 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -586,6 +586,8 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm, "virt=%s resrc=dev reason=%s %s uuid=%s %s", virt, reason, vmname, uuidstr, device); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + break; case VIR_DOMAIN_TPM_TYPE_LAST: default: break; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..21b66d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -864,7 +864,8 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, "tpm-crb")
VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, - "passthrough") + "passthrough", + "emulator")
VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, "intel") @@ -2601,6 +2602,11 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def) case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: VIR_FREE(def->data.passthrough.source.data.file.path); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + VIR_FREE(def->data.emulator.source.data.nix.path);
Why do we not need virDomainChrSourceDefFree/virObjectUnref(&def->data.emulator.source); here? (the same applies to case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH)
+ VIR_FREE(def->data.emulator.storagepath); + VIR_FREE(def->data.emulator.logfile); + break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -12585,6 +12591,11 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * </backend> * </tpm> * + * or like this: + * + * <tpm model='tpm-tis'>
[…snip] Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 07:49 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows:
<tpm model='tpm-tis'> <backend type='emulator'/> </tpm>
The XML will currently only define a TPM 1.2.
Extend the documentation.
Add a test case testing the XML parser and formatter.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- docs/formatdomain.html.in | 30 +++++++++++++++++++++++++++ docs/schemas/domaincommon.rng | 5 +++++ src/conf/domain_audit.c | 2 ++ src/conf/domain_conf.c | 28 +++++++++++++++++++------ src/conf/domain_conf.h | 7 +++++++ src/qemu/qemu_cgroup.c | 1 + src/qemu/qemu_command.c | 1 + src/qemu/qemu_domain.c | 1 + src/security/security_dac.c | 2 ++ src/security/security_selinux.c | 2 ++ tests/qemuxml2argvdata/tpm-emulator.xml | 30 +++++++++++++++++++++++++++ tests/qemuxml2xmloutdata/tpm-emulator.xml | 34 +++++++++++++++++++++++++++++++ tests/qemuxml2xmltest.c | 1 + 13 files changed, 138 insertions(+), 6 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator.xml
diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index caeb14e..4f56784 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7650,6 +7650,26 @@ qemu-kvm -net nic,model=? /dev/null </devices> ... </pre> + + <p> + The emulator device type gives access to a TPM emulator providing + TPM functionlity 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.4.0</span> + </p> + <p> + Example: usage of the TPM Emulator + </p> +<pre> + ... + <devices> + <tpm model='tpm-tis'> + <backend type='emulator'> + </backend> + </tpm> + </devices> + ... +</pre> <dl> <dt><code>model</code></dt> <dd> @@ -7683,6 +7703,16 @@ qemu-kvm -net nic,model=? /dev/null </p> </dd> </dl> + <dl> + <dt><code>emulator</code></dt> + <dd> + <p> + For this backend type the 'swtpm' TPM Emulator must be installed on the + host. Libvirt will automatically start an independent TPM emulator + for each QEMU guest requesting access to it. + </p> + </dd> + </dl> </dd> </dl>
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 0a6b29b..a9a1020 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4137,6 +4137,11 @@ </attribute> <ref name="tpm-passthrough-device"/> </group> + <group> + <attribute name="type"> + <value>emulator</value> + </attribute> + </group> </choice> </element> </define> diff --git a/src/conf/domain_audit.c b/src/conf/domain_audit.c index 82868bc..25cccdd 100644 --- a/src/conf/domain_audit.c +++ b/src/conf/domain_audit.c @@ -586,6 +586,8 @@ virDomainAuditTPM(virDomainObjPtr vm, virDomainTPMDefPtr tpm, "virt=%s resrc=dev reason=%s %s uuid=%s %s", virt, reason, vmname, uuidstr, device); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + break; case VIR_DOMAIN_TPM_TYPE_LAST: default: break; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index f678e26..21b66d7 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -864,7 +864,8 @@ VIR_ENUM_IMPL(virDomainTPMModel, VIR_DOMAIN_TPM_MODEL_LAST, "tpm-crb")
VIR_ENUM_IMPL(virDomainTPMBackend, VIR_DOMAIN_TPM_TYPE_LAST, - "passthrough") + "passthrough", + "emulator")
VIR_ENUM_IMPL(virDomainIOMMUModel, VIR_DOMAIN_IOMMU_MODEL_LAST, "intel") @@ -2601,6 +2602,11 @@ void virDomainTPMDefFree(virDomainTPMDefPtr def) case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: VIR_FREE(def->data.passthrough.source.data.file.path); break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + VIR_FREE(def->data.emulator.source.data.nix.path); Why do we not need virDomainChrSourceDefFree/virObjectUnref(&def->data.emulator.source); here? (the same applies to case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH)
The are declared as embedded structures rather than pointers. The only other similar case is Shmem. union { struct { virDomainChrSourceDef source; } passthrough; struct { virDomainChrSourceDef source; char *storagepath; char *logfile; } emulator; } data; We should call virDomainChrSourceDefClear() rather than VIR_FREE() directly. The end result is the same, though. Fixed. I will fix the passthrough case later. Stefan

Extend the QEMU capabilities with tpm-emulator support. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_capabilities.c | 5 +++++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 1 + 7 files changed, 11 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 920a596..961cd2d 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -475,6 +475,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "disk-write-cache", "nbd-tls", "tpm-crb", + "tpm-emulator", ); @@ -2338,6 +2339,10 @@ static const struct tpmTypeToCaps virQEMUCapsTPMTypesToCaps[] = { .type = VIR_DOMAIN_TPM_TYPE_PASSTHROUGH, .caps = QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, }, + { + .type = VIR_DOMAIN_TPM_TYPE_EMULATOR, + .caps = QEMU_CAPS_DEVICE_TPM_EMULATOR, + }, }; const struct tpmTypeToCaps virQEMUCapsTPMModelsToCaps[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index b9628b8..7092fbe 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -459,6 +459,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ QEMU_CAPS_NBD_TLS, /* NBD server supports TLS transport */ QEMU_CAPS_DEVICE_TPM_CRB, /* -device tpm-crb */ + QEMU_CAPS_DEVICE_TPM_EMULATOR, /* -tpmdev emulator */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index 64bd554..fd981f4 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -119,6 +119,7 @@ <flag name='seccomp-blacklist'/> <flag name='disk-write-cache'/> <flag name='nbd-tls'/> + <flag name='tpm-emulator'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342058</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml index 197060a..6349d36 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml @@ -158,6 +158,7 @@ <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> <flag name='nbd-tls'/> + <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342346</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml index b0eb055..743a1aa 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml @@ -155,6 +155,7 @@ <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> <flag name='nbd-tls'/> + <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>419215</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index 80f3ec6..bc98d6e 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -120,6 +120,7 @@ <flag name='query-cpus-fast'/> <flag name='disk-write-cache'/> <flag name='nbd-tls'/> + <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>0</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml index 150fbd2..665959a 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml @@ -196,6 +196,7 @@ <flag name='disk-write-cache'/> <flag name='nbd-tls'/> <flag name='tpm-crb'/> + <flag name='tpm-emulator'/> <version>2011090</version> <kvmVersion>0</kvmVersion> <microcodeVersion>390060</microcodeVersion> -- 2.5.5

Implement virFileChownFiles() which changes file ownership of all files in a given directory. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- src/libvirt_private.syms | 1 + src/util/virfile.c | 55 ++++++++++++++++++++++++++++++++++++++++++++++++ src/util/virfile.h | 3 +++ 3 files changed, 59 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f8883dc..75b8932 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1761,6 +1761,7 @@ virFileActivateDirOverride; virFileBindMountDevice; virFileBuildPath; virFileCanonicalizePath; +virFileChownFiles; virFileClose; virFileComparePaths; virFileCopyACLs; diff --git a/src/util/virfile.c b/src/util/virfile.c index 523241f..629aa67 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -2990,6 +2990,61 @@ void virDirClose(DIR **dirp) *dirp = NULL; } + +/* + * virFileChownFiles: + * @name: name of the directory + * @uid: uid + * @gid: gid + * + * Change ownership of all regular files in a directory. + * + * Returns -1 on error, with error already reported, 0 on success. + */ +int virFileChownFiles(const char *name, + uid_t uid, + gid_t gid) +{ + struct dirent *ent; + int ret = -1; + int direrr; + DIR *dir; + char *path = NULL; + + if (virDirOpen(&dir, name) < 0) + return -1; + + while ((direrr = virDirRead(dir, &ent, name)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&path, "%s/%s", name, ent->d_name) < 0) + goto cleanup; + + if (chown(path, uid, gid) < 0) { + virReportSystemError(errno, + _("cannot chown '%s' to (%u, %u)"), + ent->d_name, (unsigned int) uid, + (unsigned int) gid); + goto cleanup; + } + VIR_FREE(path); + } + + if (direrr < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(path); + + virDirClose(&dir); + + return ret; +} + + static int virFileMakePathHelper(char *path, mode_t mode) { diff --git a/src/util/virfile.h b/src/util/virfile.h index 6b0cbad..c7a32c3 100644 --- a/src/util/virfile.h +++ b/src/util/virfile.h @@ -238,6 +238,9 @@ int virFileOpenAs(const char *path, int openflags, mode_t mode, ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; int virFileRemove(const char *path, uid_t uid, gid_t gid); +int virFileChownFiles(const char *name, uid_t uid, gid_t gid) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK; + enum { VIR_DIR_CREATE_NONE = 0, VIR_DIR_CREATE_AS_UID = (1 << 0), -- 2.5.5

Extend the DAC and SELinux modules with support for the tpm-emulator. We label the Unix socket that QEMU connects to after starting swtmp with DAC and SELinux labels. We do not have to restore the labels in this case since the tpm-emulator will remove the Unix socket when it terminates. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- src/security/security_dac.c | 5 +++++ src/security/security_selinux.c | 6 ++++++ 2 files changed, 11 insertions(+) diff --git a/src/security/security_dac.c b/src/security/security_dac.c index 3ab2299..4b623dc 100644 --- a/src/security/security_dac.c +++ b/src/security/security_dac.c @@ -1373,6 +1373,10 @@ virSecurityDACSetTPMFileLabel(virSecurityManagerPtr mgr, false); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = virSecurityDACSetChardevLabel(mgr, def, + &tpm->data.emulator.source, + false); + break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -1395,6 +1399,7 @@ virSecurityDACRestoreTPMFileLabel(virSecurityManagerPtr mgr, false); break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + /* swtpm will have removed the Unix socket upon termination */ case VIR_DOMAIN_TPM_TYPE_LAST: break; } diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 5d20fda..92e8415 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -1473,6 +1473,11 @@ virSecuritySELinuxSetTPMFileLabel(virSecurityManagerPtr mgr, } break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + tpmdev = tpm->data.emulator.source.data.nix.path; + rc = virSecuritySELinuxSetFilecon(mgr, tpmdev, seclabel->imagelabel); + if (rc < 0) + return -1; + break; case VIR_DOMAIN_TPM_TYPE_LAST: break; } @@ -1507,6 +1512,7 @@ virSecuritySELinuxRestoreTPMFileLabelInt(virSecurityManagerPtr mgr, } break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + /* swtpm will have removed the Unix socket upon termination */ case VIR_DOMAIN_TPM_TYPE_LAST: break; } -- 2.5.5

Extend qemu_conf with user and group for running the tpm-emulator and add directories to the configuration for the locations of the log, state, and socket of the tpm-emulator. Also add these new directories to the QEMU Makefile.inc.am and the RPM spec file libvirt.spec.in. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- libvirt.spec.in | 2 ++ src/qemu/Makefile.inc.am | 6 ++++++ src/qemu/libvirtd_qemu.aug | 5 +++++ src/qemu/qemu.conf | 8 +++++++ src/qemu/qemu_conf.c | 43 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 6 ++++++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 7 files changed, 72 insertions(+) diff --git a/libvirt.spec.in b/libvirt.spec.in index 9ea5e6b..cd24453 100644 --- a/libvirt.spec.in +++ b/libvirt.spec.in @@ -1900,6 +1900,8 @@ exit 0 %{_datadir}/augeas/lenses/libvirtd_qemu.aug %{_datadir}/augeas/lenses/tests/test_libvirtd_qemu.aug %{_libdir}/%{name}/connection-driver/libvirt_driver_qemu.so +%dir %attr(0711, root, root) %{_localstatedir}/lib/libvirt/swtpm/ +%dir %attr(0711, root, root) %{_localstatedir}/log/swtpm/libvirt/qemu/ %endif %if %{with_lxc} diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 63e7c87..7f50501 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -129,12 +129,18 @@ install-data-qemu: $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/libvirt/qemu" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/lib/libvirt/swtpm" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/run/libvirt/qemu/swtpm" + $(MKDIR_P) "$(DESTDIR)$(localstatedir)/log/swtpm/libvirt/qemu" uninstall-data-qemu: rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/qemu" ||: rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/qemu" ||: rmdir "$(DESTDIR)$(localstatedir)/cache/libvirt/qemu" ||: rmdir "$(DESTDIR)$(localstatedir)/log/libvirt/qemu" ||: + rmdir "$(DESTDIR)$(localstatedir)/lib/libvirt/swtpm" + rmdir "$(DESTDIR)$(localstatedir)/run/libvirt/qemu/swtpm" ||: + rmdir "$(DESTDIR)$(localstatedir)/log/swtpm/libvirt/qemu" ||: endif WITH_QEMU diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug index c19bf3a..23bfe67 100644 --- a/src/qemu/libvirtd_qemu.aug +++ b/src/qemu/libvirtd_qemu.aug @@ -118,6 +118,9 @@ module Libvirtd_qemu = let vxhs_entry = bool_entry "vxhs_tls" | str_entry "vxhs_tls_x509_cert_dir" + let swtpm_user_entry = str_entry "swtpm_user" + let swtpm_group_entry = str_entry "swtpm_group" + (* Each entry in the config is one of the following ... *) let entry = default_tls_entry | vnc_entry @@ -137,6 +140,8 @@ module Libvirtd_qemu = | gluster_debug_level_entry | memory_entry | vxhs_entry + | swtpm_user_entry + | swtpm_group_entry let comment = [ label "#comment" . del /#[ \t]*/ "# " . store /([^ \t\n][^\n]*)?/ . del /\n/ "\n" ] let empty = [ label "#empty" . eol ] diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf index 3444185..26a6dc7 100644 --- a/src/qemu/qemu.conf +++ b/src/qemu/qemu.conf @@ -779,3 +779,11 @@ # This directory is used for memoryBacking source if configured as file. # NOTE: big files will be stored here #memory_backing_dir = "/var/lib/libvirt/qemu/ram" + +# User for the swtpm TPM Emulator +# +# Default is 'tss'; this is the same user that tcsd (TrouSerS) installs +# and uses; alternative is 'root' +# +#swtpm_user = "tss" +#swtpm_group = "tss" diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index bfbb572..5383fd2 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -159,6 +159,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) "%s/log/libvirt/qemu", LOCALSTATEDIR) < 0) goto error; + if (virAsprintf(&cfg->swtpmLogDir, + "%s/log/swtpm/libvirt/qemu", LOCALSTATEDIR) < 0) + goto error; + if (VIR_STRDUP(cfg->configBaseDir, SYSCONFDIR "/libvirt") < 0) goto error; @@ -166,6 +170,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) "%s/run/libvirt/qemu", LOCALSTATEDIR) < 0) goto error; + if (virAsprintf(&cfg->swtpmStateDir, + "%s/run/libvirt/qemu/swtpm", LOCALSTATEDIR) < 0) + goto error; + if (virAsprintf(&cfg->cacheDir, "%s/cache/libvirt/qemu", LOCALSTATEDIR) < 0) goto error; @@ -186,6 +194,13 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; if (virAsprintf(&cfg->memoryBackingDir, "%s/ram", cfg->libDir) < 0) goto error; + if (virAsprintf(&cfg->swtpmStorageDir, "%s/lib/libvirt/swtpm", + LOCALSTATEDIR) < 0) + goto error; + if (virGetUserID("tss", &cfg->swtpm_user) < 0) + cfg->swtpm_user = 0; /* fall back to root */ + if (virGetGroupID("tss", &cfg->swtpm_group) < 0) + cfg->swtpm_group = 0; /* fall back to root */ } else { char *rundir; char *cachedir; @@ -199,6 +214,11 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) VIR_FREE(cachedir); goto error; } + if (virAsprintf(&cfg->swtpmLogDir, + "%s/qemu/log", cachedir) < 0) { + VIR_FREE(cachedir); + goto error; + } if (virAsprintf(&cfg->cacheDir, "%s/qemu/cache", cachedir) < 0) { VIR_FREE(cachedir); goto error; @@ -214,6 +234,9 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) } VIR_FREE(rundir); + if (virAsprintf(&cfg->swtpmStateDir, "%s/swtpm", cfg->stateDir) < 0) + goto error; + if (!(cfg->configBaseDir = virGetUserConfigDirectory())) goto error; @@ -233,6 +256,10 @@ virQEMUDriverConfigPtr virQEMUDriverConfigNew(bool privileged) goto error; if (virAsprintf(&cfg->memoryBackingDir, "%s/qemu/ram", cfg->configBaseDir) < 0) goto error; + if (virAsprintf(&cfg->swtpmStorageDir, "%s/qemu/swtpm", cfg->configBaseDir) < 0) + goto error; + cfg->swtpm_user = (uid_t)-1; + cfg->swtpm_group = (gid_t)-1; } if (virAsprintf(&cfg->configDir, "%s/qemu", cfg->configBaseDir) < 0) @@ -351,7 +378,9 @@ static void virQEMUDriverConfigDispose(void *obj) VIR_FREE(cfg->configDir); VIR_FREE(cfg->autostartDir); VIR_FREE(cfg->logDir); + VIR_FREE(cfg->swtpmLogDir); VIR_FREE(cfg->stateDir); + VIR_FREE(cfg->swtpmStateDir); VIR_FREE(cfg->libDir); VIR_FREE(cfg->cacheDir); @@ -400,6 +429,7 @@ static void virQEMUDriverConfigDispose(void *obj) virFirmwareFreeList(cfg->firmwares, cfg->nfirmwares); VIR_FREE(cfg->memoryBackingDir); + VIR_FREE(cfg->swtpmStorageDir); } @@ -471,6 +501,7 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, size_t i, j; char *stdioHandler = NULL; char *user = NULL, *group = NULL; + char *swtpm_user = NULL, *swtpm_group = NULL; char **controllers = NULL; char **hugetlbfs = NULL; char **nvram = NULL; @@ -907,6 +938,16 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, if (virConfGetValueString(conf, "memory_backing_dir", &cfg->memoryBackingDir) < 0) goto cleanup; + if (virConfGetValueString(conf, "swtpm_user", &swtpm_user) < 0) + goto cleanup; + if (swtpm_user && virGetUserID(swtpm_user, &cfg->swtpm_user) < 0) + goto cleanup; + + if (virConfGetValueString(conf, "swtpm_group", &swtpm_group) < 0) + goto cleanup; + if (swtpm_group && virGetGroupID(swtpm_group, &cfg->swtpm_group) < 0) + goto cleanup; + ret = 0; cleanup: @@ -917,6 +958,8 @@ int virQEMUDriverConfigLoadFile(virQEMUDriverConfigPtr cfg, VIR_FREE(corestr); VIR_FREE(user); VIR_FREE(group); + VIR_FREE(swtpm_user); + VIR_FREE(swtpm_group); virConfFree(conf); return ret; } diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h index e1ad546..19dc0bc 100644 --- a/src/qemu/qemu_conf.h +++ b/src/qemu/qemu_conf.h @@ -102,7 +102,9 @@ struct _virQEMUDriverConfig { char *configDir; char *autostartDir; char *logDir; + char *swtpmLogDir; char *stateDir; + char *swtpmStateDir; /* These two directories are ones QEMU processes use (so must match * the QEMU user/group */ char *libDir; @@ -111,6 +113,7 @@ struct _virQEMUDriverConfig { char *snapshotDir; char *channelTargetDir; char *nvramDir; + char *swtpmStorageDir; char *defaultTLSx509certdir; bool checkdefaultTLSx509certdir; @@ -206,6 +209,9 @@ struct _virQEMUDriverConfig { bool vxhsTLS; char *vxhsTLSx509certdir; + + uid_t swtpm_user; + gid_t swtpm_group; }; /* Main driver state */ diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in index 688e5b9..6d6e1d4 100644 --- a/src/qemu/test_libvirtd_qemu.aug.in +++ b/src/qemu/test_libvirtd_qemu.aug.in @@ -100,3 +100,5 @@ module Test_libvirtd_qemu = { "1" = "mount" } } { "memory_backing_dir" = "/var/lib/libvirt/qemu/ram" } +{ "swtpm_user" = "tss" } +{ "swtpm_group" = "tss" } -- 2.5.5

On 05/10/2018 05:57 PM, Stefan Berger wrote:
Extend qemu_conf with user and group for running the tpm-emulator and add directories to the configuration for the locations of the log, state, and socket of the tpm-emulator.
Also add these new directories to the QEMU Makefile.inc.am and the RPM spec file libvirt.spec.in.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- libvirt.spec.in | 2 ++ src/qemu/Makefile.inc.am | 6 ++++++ src/qemu/libvirtd_qemu.aug | 5 +++++ src/qemu/qemu.conf | 8 +++++++ src/qemu/qemu_conf.c | 43 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 6 ++++++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 7 files changed, 72 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com> John although I still cannot test using make rpm/distcheck locally /-| - not because of these changes, but something else.

On 05/15/2018 07:41 AM, John Ferlan wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
Extend qemu_conf with user and group for running the tpm-emulator and add directories to the configuration for the locations of the log, state, and socket of the tpm-emulator.
Also add these new directories to the QEMU Makefile.inc.am and the RPM spec file libvirt.spec.in.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- libvirt.spec.in | 2 ++ src/qemu/Makefile.inc.am | 6 ++++++ src/qemu/libvirtd_qemu.aug | 5 +++++ src/qemu/qemu.conf | 8 +++++++ src/qemu/qemu_conf.c | 43 ++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_conf.h | 6 ++++++ src/qemu/test_libvirtd_qemu.aug.in | 2 ++ 7 files changed, 72 insertions(+)
Reviewed-by: John Ferlan <jferlan@redhat.com>
John
although I still cannot test using make rpm/distcheck locally /-| - not because of these changes, but something else.
Works (probably only after the changes I made because I hadn't tried earlier.) Stefan

Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device. Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined. This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/Makefile.inc.am | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 154 ++++++++++ src/qemu/qemu_extdevice.h | 53 ++++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 12 + src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_tpm.h | 50 +++ 9 files changed, 1036 insertions(+) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 7f50501..46797af 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_domain_address.h \ qemu/qemu_cgroup.c \ qemu/qemu_cgroup.h \ + qemu/qemu_extdevice.c \ + qemu/qemu_extdevice.h \ qemu/qemu_hostdev.c \ qemu/qemu_hostdev.h \ qemu/qemu_hotplug.c \ @@ -51,6 +53,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_security.h \ qemu/qemu_qapi.c \ qemu/qemu_qapi.h \ + qemu/qemu_tpm.c \ + qemu/qemu_tpm.h \ $(NULL) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 774a102..e2d2a24 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -34,6 +34,7 @@ #include "qemu_migration.h" #include "qemu_migration_params.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -7174,6 +7175,7 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, VIR_WARN("unable to remove snapshot directory %s", snapDir); VIR_FREE(snapDir); } + qemuExtDevicesCleanupHost(driver, vm->def); virDomainObjListRemove(driver->domains, vm); diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b03eb30..b576a4d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -60,6 +60,7 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "virerror.h" #include "virlog.h" @@ -7558,6 +7559,10 @@ qemuDomainUndefineFlags(virDomainPtr dom, if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0) goto endjob; + /* in case domain is NOT running, remove any TPM storage */ + if (!vm->persistent) + qemuExtDevicesCleanupHost(driver, vm->def); + event = virDomainEventLifecycleNewFromObj(vm, VIR_DOMAIN_EVENT_UNDEFINED, VIR_DOMAIN_EVENT_UNDEFINED_REMOVED); diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c new file mode 100644 index 0000000..790b19b --- /dev/null +++ b/src/qemu/qemu_extdevice.c @@ -0,0 +1,154 @@ +/* + * qemu_extdevice.c: QEMU external devices support + * + * Copyright (C) 2014, 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ + +#include <config.h> + +#include "qemu_extdevice.h" +#include "qemu_domain.h" +#include "qemu_tpm.h" + +#include "viralloc.h" +#include "virlog.h" +#include "virstring.h" +#include "virtime.h" + +#define VIR_FROM_THIS VIR_FROM_QEMU + +VIR_LOG_INIT("qemu.qemu_extdevice") + +int +qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, + virCommandPtr cmd, + const char *info) +{ + int ret = -1; + char *timestamp = NULL; + char *logline = NULL; + int logFD; + + logFD = qemuDomainLogContextGetWriteFD(logCtxt); + + if ((timestamp = virTimeStringNow()) == NULL) + goto cleanup; + + if (virAsprintf(&logline, "%s: Starting external device: %s\n", + timestamp, info) < 0) + goto cleanup; + + if (safewrite(logFD, logline, strlen(logline)) < 0) + goto cleanup; + + virCommandWriteArgLog(cmd, logFD); + + ret = 0; + + cleanup: + VIR_FREE(timestamp); + VIR_FREE(logline); + + return ret; +} + + +/* + * qemuExtDevicesInitPaths: + * + * @driver: QEMU driver + * @def: domain definition + * + * Initialize paths of external devices so that it is known where state is + * stored and we can remove directories and files in case of domain XML + * changes. + */ +static int +qemuExtDevicesInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + int ret = 0; + + if (def->tpm) + ret = qemuExtTPMInitPaths(driver, def); + + return ret; +} + + +/* + * qemuExtDevicesPrepareHost: + * + * @driver: QEMU driver + * @def: domain definition + * + * Prepare host storage paths for external devices. + */ +int +qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + int ret = 0; + + if (def->tpm) + ret = qemuExtTPMPrepareHost(driver, def); + + return ret; +} + + +void +qemuExtDevicesCleanupHost(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + if (qemuExtDevicesInitPaths(driver, def) < 0) + return; + + if (def->tpm) + qemuExtTPMCleanupHost(def); +} + + +int +qemuExtDevicesStart(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) +{ + int ret = 0; + + if (qemuExtDevicesInitPaths(driver, def) < 0) + return -1; + + if (def->tpm) + ret = qemuExtTPMStart(driver, def, logCtxt); + + return ret; +} + + +void +qemuExtDevicesStop(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + if (qemuExtDevicesInitPaths(driver, def) < 0) + return; + + if (def->tpm) + qemuExtTPMStop(driver, def); +} diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h new file mode 100644 index 0000000..6de858b --- /dev/null +++ b/src/qemu/qemu_extdevice.h @@ -0,0 +1,53 @@ +/* + * qemu_extdevice.h: QEMU external devices support + * + * Copyright (C) 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ +#ifndef __QEMU_EXTDEVICE_H__ +# define __QEMU_EXTDEVICE_H__ + +# include "qemu_conf.h" +# include "qemu_domain.h" + +int qemuExtDeviceLogCommand(qemuDomainLogContextPtr logCtxt, + virCommandPtr cmd, + const char *info) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +int qemuExtDevicesPrepareHost(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + +void qemuExtDevicesCleanupHost(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int qemuExtDevicesStart(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +void qemuExtDevicesStop(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +#endif /* __QEMU_EXTDEVICE_H__ */ diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index f01bb64..6827aea 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -39,6 +39,7 @@ #include "qemu_hotplug.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "domain_audit.h" #include "virlog.h" @@ -2920,6 +2921,7 @@ qemuMigrationSrcConfirm(virQEMUDriverPtr driver, if (!virDomainObjIsActive(vm)) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) { virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); + qemuExtDevicesCleanupHost(driver, vm->def); vm->persistent = 0; } qemuDomainRemoveInactiveJob(driver, vm); @@ -4521,6 +4523,7 @@ qemuMigrationSrcPerformJob(virQEMUDriverPtr driver, if (!virDomainObjIsActive(vm) && ret == 0) { if (flags & VIR_MIGRATE_UNDEFINE_SOURCE) { virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm); + qemuExtDevicesCleanupHost(driver, vm->def); vm->persistent = 0; } qemuDomainRemoveInactiveJob(driver, vm); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 37876b8..9370de3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -48,6 +48,7 @@ #include "qemu_migration_params.h" #include "qemu_interface.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "cpu/cpu.h" #include "datatypes.h" @@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0) goto cleanup; + VIR_DEBUG("Preparing external devices"); + if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); @@ -5955,6 +5960,9 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; logfile = qemuDomainLogContextGetWriteFD(logCtxt); + if (qemuExtDevicesStart(driver, vm->def, logCtxt) < 0) + goto cleanup; + VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(driver, qemuDomainLogContextGetManager(logCtxt), @@ -6194,6 +6202,8 @@ qemuProcessLaunch(virConnectPtr conn, ret = 0; cleanup: + if (ret) + qemuExtDevicesStop(driver, vm->def); qemuDomainSecretDestroy(vm); virCommandFree(cmd); virObjectUnref(logCtxt); @@ -6614,6 +6624,8 @@ void qemuProcessStop(virQEMUDriverPtr driver, qemuDomainCleanupRun(driver, vm); + qemuExtDevicesStop(driver, vm->def); + /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm); diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c new file mode 100644 index 0000000..024d24d --- /dev/null +++ b/src/qemu/qemu_tpm.c @@ -0,0 +1,753 @@ +/* + * qemu_tpm.c: QEMU TPM support + * + * Copyright (C) 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ + +#include <config.h> + +#include <sys/types.h> +#include <unistd.h> +#include <fcntl.h> +#include <cap-ng.h> + +#include "qemu_extdevice.h" +#include "qemu_domain.h" + +#include "conf/domain_conf.h" +#include "vircommand.h" +#include "viralloc.h" +#include "virkmod.h" +#include "virlog.h" +#include "virutil.h" +#include "viruuid.h" +#include "virfile.h" +#include "virstring.h" +#include "configmake.h" +#include "qemu_tpm.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 + * + * @swtpmStorageDir: directory for swtpm persistent state + * @vmname: The name of the VM for which to create the storage + * + * Create the swtpm's storage path + */ +static char * +qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, + const char *vmname) +{ + char *path = NULL; + + ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname)); + + return path; +} + + +/* + * virtTPMGetTPMStorageDir: + * + * @storagepath: directory for swtpm's persistent state + * + * Derive the 'TPMStorageDir' from the storagepath by searching + * for the last '/'. + */ +static char * +qemuTPMGetTPMStorageDir(const char *storagepath) +{ + const char *tail = strrchr(storagepath, '/'); + char *path = NULL; + + if (!tail) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not get tail of storagedir %s"), + storagepath); + return NULL; + } + ignore_value(VIR_STRNDUP(path, storagepath, tail - storagepath)); + + return path; +} + + +/* + * qemuTPMEmulatorInitStorage + * + * Initialize the TPM Emulator storage by creating its root directory, + * which is typically found in /var/lib/libvirt/tpm. + * + */ +static int +qemuTPMEmulatorInitStorage(const char *swtpmStorageDir) +{ + int rc = 0; + + /* allow others to cd into this dir */ + if (virFileMakePathWithMode(swtpmStorageDir, 0711) < 0) { + virReportSystemError(errno, + _("Could not create TPM directory %s"), + swtpmStorageDir); + rc = -1; + } + + return rc; +} + + +/* + * qemuTPMCreateEmulatorStorage + * + * @storagepath: directory for swtpm's pesistent state + * @created: a pointer to a bool that will be set to true if the + * storage was created because it did not exist yet + * @swtpm_user: The uid that needs to be able to access the directory + * @swtpm_group: The gid that needs to be able to access the directory + * + * Unless the storage path for the swtpm for the given VM + * already exists, create it and make it accessible for the given userid. + * Adapt ownership of the directory and all swtpm's state files there. + */ +static int +qemuTPMCreateEmulatorStorage(const char *storagepath, + bool *created, + uid_t swtpm_user, + gid_t swtpm_group) +{ + int ret = -1; + char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath); + + if (!swtpmStorageDir) + return -1; + + if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0) + goto cleanup; + + *created = false; + + if (!virFileExists(storagepath)) + *created = true; + + if (virDirCreate(storagepath, 0700, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not create directory %s as %u:%d"), + storagepath, swtpm_user, swtpm_group); + goto cleanup; + } + + if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(swtpmStorageDir); + + return ret; +} + + +static void +qemuTPMDeleteEmulatorStorage(virDomainTPMDefPtr tpm) +{ + char *path = qemuTPMGetTPMStorageDir(tpm->data.emulator.storagepath); + + if (path) { + ignore_value(virFileDeleteTree(path)); + VIR_FREE(path); + } +} + + +/* + * qemuTPMCreateEmulatorSocket: + * + * @swtpmStateDir: the directory where to create the socket in + * @shortName: short and unique name of the domain + * + * Create the Unix socket path from the given parameters + */ +static char * +qemuTPMCreateEmulatorSocket(const char *swtpmStateDir, + const char *shortName) +{ + char *path = NULL; + + ignore_value(virAsprintf(&path, "%s/%s-swtpm.sock", swtpmStateDir, + shortName)); + + return path; +} + + +/* + * qemuTPMEmulatorInitPaths: + * + * @tpm: TPM definition for an emulator type + * @swtpmStorageDir: the general swtpm storage dir which is used as a base + * directory for creating VM specific directories + * @uuid: the UUID of the VM + */ +static int +qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm, + const char *swtpmStorageDir, + const unsigned char *uuid) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + virUUIDFormat(uuid, uuidstr); + + if (!tpm->data.emulator.storagepath && + !(tpm->data.emulator.storagepath = + qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr))) + return -1; + + return 0; +} + + +/* + * qemuTPMEmulatorPrepareHost: + * + * @tpm: tpm definition + * @logDir: directory where swtpm writes its logs into + * @vmname: name of the VM + * @swtpm_user: uid to run the swtpm with + * @swtpm_group: gid to run the swtpm with + * @swtpmStateDir: directory for swtpm's persistent state + * @qemu_user: uid that qemu will run with; we share the socket file with it + * @shortName: short and unique name of the domain + * + * Prepare the log directory for the swtpm and adjust ownership of it and the + * log file we will be using. Prepare the state directory where we will share + * the socket between tss and qemu users. + */ +static int +qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, + const char *logDir, + const char *vmname, + uid_t swtpm_user, + gid_t swtpm_group, + const char *swtpmStateDir, + uid_t qemu_user, + const char *shortName) +{ + int ret = -1; + + if (qemuTPMEmulatorInit() < 0) + return -1; + + /* create log dir ... */ + if (virFileMakePathWithMode(logDir, 0730) < 0) + return -1; + + /* ... and adjust ownership */ + if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup; + + /* create logfile name ... */ + if (!tpm->data.emulator.logfile && + virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log", + logDir, vmname) < 0) + goto cleanup; + + /* ... and make sure it can be accessed by swtpm_user */ + if (virFileExists(tpm->data.emulator.logfile) && + chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { + virReportSystemError(errno, + _("Could not chown on swtpm logfile %s"), + tpm->data.emulator.logfile); + goto cleanup; + } + + /* + create our swtpm state dir ... + - QEMU user needs to be able to access the socket there + - swtpm group needs to be able to create files there + - in privileged mode 0570 would be enough, for non-privileged mode + we need 0770 + */ + if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup; + + /* create the socket filename */ + if (!tpm->data.emulator.source.data.nix.path && + !(tpm->data.emulator.source.data.nix.path = + qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + goto cleanup; + tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + + ret = 0; + + cleanup: + + return ret; +} + + +/* + * qemuTPMEmulatorRunSetup + * + * @storagepath: path to the directory for TPM state + * @vmname: the name of the VM + * @vmuuid: the UUID of the VM + * @privileged: whether we are running in privileged mode + * @swtpm_user: The userid to switch to when setting up the TPM; + * typically this should be the uid of 'tss' or 'root' + * @swtpm_group: The group id to switch to + * @logfile: The file to write the log into; it must be writable + * for the user given by userid or 'tss' + * + * Setup the external swtpm by creating endorsement key and + * certificates for it. + */ +static int +qemuTPMEmulatorRunSetup(const char *storagepath, + const char *vmname, + const unsigned char *vmuuid, + bool privileged, + uid_t swtpm_user, + gid_t swtpm_group, + const char *logfile) +{ + virCommandPtr cmd = NULL; + int exitstatus; + int ret = -1; + char uuid[VIR_UUID_STRING_BUFLEN]; + char *vmid = NULL; + + if (!privileged) + return virFileWriteStr(logfile, + _("Did not create EK and certificates since " + "this requires privileged mode\n"), + 0600); + + cmd = virCommandNew(swtpm_setup); + if (!cmd) + goto cleanup; + + virUUIDFormat(vmuuid, uuid); + if (virAsprintf(&vmid, "%s:%s", vmname, uuid) < 0) + goto cleanup; + + virCommandSetUID(cmd, swtpm_user); + virCommandSetGID(cmd, swtpm_group); + + virCommandAddArgList(cmd, + "--tpm-state", storagepath, + "--vmid", vmid, + "--logfile", logfile, + "--createek", + "--create-ek-cert", + "--create-platform-cert", + "--lock-nvram", + "--not-overwrite", + NULL); + + virCommandClearCaps(cmd); + + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not run '%s'. exitstatus: %d; " + "Check error log '%s' for details."), + swtpm_setup, exitstatus, logfile); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(vmid); + virCommandFree(cmd); + + return ret; +} + + +/* + * qemuTPMEmulatorBuildCommand: + * + * @tpm: TPM definition + * @vmname: The name of the VM + * @vmuuid: The UUID of the VM + * @privileged: whether we are running in privileged mode + * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root) + * @swtpm_group: The gid for the swtpm to run as + * + * Create the virCommand use for starting the emulator + * Do some initializations on the way, such as creation of storage + * and emulator setup. + */ +static virCommandPtr +qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, + const char *vmname, + const unsigned char *vmuuid, + bool privileged, + uid_t swtpm_user, + gid_t swtpm_group) +{ + virCommandPtr cmd = NULL; + bool created = false; + + if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, + &created, swtpm_user, swtpm_group) < 0) + return NULL; + + if (created && + qemuTPMEmulatorRunSetup(tpm->data.emulator.storagepath, vmname, vmuuid, + privileged, swtpm_user, swtpm_group, + tpm->data.emulator.logfile) < 0) + goto error; + + unlink(tpm->data.emulator.source.data.nix.path); + + cmd = virCommandNew(swtpm_path); + if (!cmd) + goto error; + + virCommandClearCaps(cmd); + + virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL); + virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600", + tpm->data.emulator.source.data.nix.path); + + virCommandAddArg(cmd, "--tpmstate"); + virCommandAddArgFormat(cmd, "dir=%s,mode=0600", + tpm->data.emulator.storagepath); + + virCommandAddArg(cmd, "--log"); + virCommandAddArgFormat(cmd, "file=%s", tpm->data.emulator.logfile); + + virCommandSetUID(cmd, swtpm_user); + virCommandSetGID(cmd, swtpm_group); + + return cmd; + + error: + if (created) + qemuTPMDeleteEmulatorStorage(tpm); + + virCommandFree(cmd); + + return NULL; +} + + +/* + * qemuTPMEmulatorStop + * @swtpmStateDir: A directory where the socket is located + * @shortName: short and unique name of the domain + * + * Gracefully stop the swptm + */ +static void +qemuTPMEmulatorStop(const char *swtpmStateDir, + const char *shortName) +{ + virCommandPtr cmd; + char *pathname; + char *errbuf = NULL; + + if (qemuTPMEmulatorInit() < 0) + return; + + if (!(pathname = qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + return; + + if (!virFileExists(pathname)) + goto cleanup; + + cmd = virCommandNew(swtpm_ioctl); + if (!cmd) + goto cleanup; + + virCommandAddArgList(cmd, "--unix", pathname, "-s", NULL); + + virCommandSetErrorBuffer(cmd, &errbuf); + + ignore_value(virCommandRun(cmd, NULL)); + + virCommandFree(cmd); + + /* clean up the socket */ + unlink(pathname); + + cleanup: + VIR_FREE(pathname); + VIR_FREE(errbuf); +} + + +int +qemuExtTPMInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir, + def->uuid); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + virObjectUnref(cfg); + + return ret; +} + + +int +qemuExtTPMPrepareHost(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = 0; + char *shortName = NULL; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + shortName = virDomainDefGetShortName(def); + if (!shortName) + goto cleanup; + + ret = qemuTPMEmulatorPrepareHost(def->tpm, cfg->swtpmLogDir, + def->name, cfg->swtpm_user, + cfg->swtpm_group, + cfg->swtpmStateDir, cfg->user, + shortName); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + cleanup: + VIR_FREE(shortName); + virObjectUnref(cfg); + + return ret; +} + + +void +qemuExtTPMCleanupHost(virDomainDefPtr def) +{ + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + qemuTPMDeleteEmulatorStorage(def->tpm); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + /* nothing to do */ + break; + } +} + + +/* + * qemuExtTPMStartEmulator: + * + * @driver: QEMU driver + * @def: domain definition + * @logCtxt: log context + * + * Start the external TPM Emulator: + * - have the command line built + * - start the external TPM Emulator and sync with it before QEMU start + */ +static int +qemuExtTPMStartEmulator(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) +{ + int ret = -1; + virCommandPtr cmd = NULL; + int exitstatus; + char *errbuf = NULL; + virQEMUDriverConfigPtr cfg; + virDomainTPMDefPtr tpm = def->tpm; + char *shortName = virDomainDefGetShortName(def); + + if (!shortName) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + /* stop any left-over TPM emulator for this VM */ + qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + + if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid, + driver->privileged, + cfg->swtpm_user, + cfg->swtpm_group))) + goto cleanup; + + if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) + goto cleanup; + + virCommandSetErrorBuffer(cmd, &errbuf); + + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " + "stderr: %s"), exitstatus, errbuf); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'swtpm'. exitstatus: %d, " + "error: %s"), exitstatus, errbuf); + goto cleanup; + } + + ret = 0; + + cleanup: + VIR_FREE(shortName); + VIR_FREE(errbuf); + virCommandFree(cmd); + + virObjectUnref(cfg); + + return ret; +} + + +int +qemuExtTPMStart(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) +{ + int ret = 0; + virDomainTPMDefPtr tpm = def->tpm; + + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = qemuExtTPMStartEmulator(driver, def, logCtxt); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +void +qemuExtTPMStop(virQEMUDriverPtr driver, + virDomainDefPtr def) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *shortName = NULL; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + shortName = virDomainDefGetShortName(def); + if (!shortName) + goto cleanup; + + qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + cleanup: + VIR_FREE(shortName); + virObjectUnref(cfg); +} diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h new file mode 100644 index 0000000..20f3a9c --- /dev/null +++ b/src/qemu/qemu_tpm.h @@ -0,0 +1,50 @@ +/* + * qemu_tpm.h: QEMU TPM support + * + * Copyright (C) 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ +#ifndef __QEMU_TPM_H__ +# define __QEMU_TPM_H__ + +# include "vircommand.h" + +int qemuExtTPMInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + +int qemuExtTPMPrepareHost(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) + ATTRIBUTE_RETURN_CHECK; + +void qemuExtTPMCleanupHost(virDomainDefPtr def) + ATTRIBUTE_NONNULL(1); + +int qemuExtTPMStart(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + +void qemuExtTPMStop(virQEMUDriverPtr driver, + virDomainDefPtr def) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +#endif /* __QEMU_TPM_H__ */ -- 2.5.5

On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/Makefile.inc.am | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 154 ++++++++++ src/qemu/qemu_extdevice.h | 53 ++++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 12 + src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_tpm.h | 50 +++ 9 files changed, 1036 insertions(+) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 7f50501..46797af 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_domain_address.h \ qemu/qemu_cgroup.c \ qemu/qemu_cgroup.h \ + qemu/qemu_extdevice.c \ + qemu/qemu_extdevice.h \ qemu/qemu_hostdev.c \ qemu/qemu_hostdev.h \ qemu/qemu_hotplug.c \ @@ -51,6 +53,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_security.h \ qemu/qemu_qapi.c \ qemu/qemu_qapi.h \ + qemu/qemu_tpm.c \ + qemu/qemu_tpm.h \ $(NULL)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 774a102..e2d2a24 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -34,6 +34,7 @@ #include "qemu_migration.h" #include "qemu_migration_params.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -7174,6 +7175,7 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, VIR_WARN("unable to remove snapshot directory %s", snapDir); VIR_FREE(snapDir); } + qemuExtDevicesCleanupHost(driver, vm->def);
virDomainObjListRemove(driver->domains, vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b03eb30..b576a4d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -60,6 +60,7 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_extdevice.h"
#include "virerror.h" #include "virlog.h" @@ -7558,6 +7559,10 @@ qemuDomainUndefineFlags(virDomainPtr dom, if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0) goto endjob;
+ /* in case domain is NOT running, remove any TPM storage */ + if (!vm->persistent) ^^^^^^^^^^^^^^^^^^^^ Can this really happen since there is a guard against this situation in the code?
“ if (!vm->persistent) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("cannot undefine transient domain")); … ” You’re validating that the domain is not persistent… but your comment says 'not running'. And why are you doing this in qemuDomainUndefineFlags and not in something like qemuDomainDestroyFlags and processMonitorEOFEvent? […snip] Haven’t looked into more detail right now. Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 08:13 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/Makefile.inc.am | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 154 ++++++++++ src/qemu/qemu_extdevice.h | 53 ++++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 12 + src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_tpm.h | 50 +++ 9 files changed, 1036 insertions(+) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h
diff --git a/src/qemu/Makefile.inc.am b/src/qemu/Makefile.inc.am index 7f50501..46797af 100644 --- a/src/qemu/Makefile.inc.am +++ b/src/qemu/Makefile.inc.am @@ -19,6 +19,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_domain_address.h \ qemu/qemu_cgroup.c \ qemu/qemu_cgroup.h \ + qemu/qemu_extdevice.c \ + qemu/qemu_extdevice.h \ qemu/qemu_hostdev.c \ qemu/qemu_hostdev.h \ qemu/qemu_hotplug.c \ @@ -51,6 +53,8 @@ QEMU_DRIVER_SOURCES = \ qemu/qemu_security.h \ qemu/qemu_qapi.c \ qemu/qemu_qapi.h \ + qemu/qemu_tpm.c \ + qemu/qemu_tpm.h \ $(NULL)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 774a102..e2d2a24 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -34,6 +34,7 @@ #include "qemu_migration.h" #include "qemu_migration_params.h" #include "qemu_security.h" +#include "qemu_extdevice.h" #include "viralloc.h" #include "virlog.h" #include "virerror.h" @@ -7174,6 +7175,7 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, VIR_WARN("unable to remove snapshot directory %s", snapDir); VIR_FREE(snapDir); } + qemuExtDevicesCleanupHost(driver, vm->def);
virDomainObjListRemove(driver->domains, vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b03eb30..b576a4d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -60,6 +60,7 @@ #include "qemu_migration_params.h" #include "qemu_blockjob.h" #include "qemu_security.h" +#include "qemu_extdevice.h"
#include "virerror.h" #include "virlog.h" @@ -7558,6 +7559,10 @@ qemuDomainUndefineFlags(virDomainPtr dom, if (virDomainDeleteConfig(cfg->configDir, cfg->autostartDir, vm) < 0) goto endjob;
+ /* in case domain is NOT running, remove any TPM storage */ + if (!vm->persistent) ^^^^^^^^^^^^^^^^^^^^ Can this really happen since there is a guard against this situation in the code?
Yes, it can. One can undefine a domain while it is running. Though the placement of this call isn't necessary (anymore, maybe was some time ago). It's being cleaned up in qemuDomainRemoveInactive(). Thanks for pointing this out. Stefan

On 05/10/2018 05:57 PM, Stefan Berger wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/Makefile.inc.am | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 154 ++++++++++ src/qemu/qemu_extdevice.h | 53 ++++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 12 + src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_tpm.h | 50 +++ 9 files changed, 1036 insertions(+) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 37876b8..9370de3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -48,6 +48,7 @@ #include "qemu_migration_params.h" #include "qemu_interface.h" #include "qemu_security.h" +#include "qemu_extdevice.h"
#include "cpu/cpu.h" #include "datatypes.h" @@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0) goto cleanup;
+ VIR_DEBUG("Preparing external devices"); + if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); @@ -5955,6 +5960,9 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; logfile = qemuDomainLogContextGetWriteFD(logCtxt);
+ if (qemuExtDevicesStart(driver, vm->def, logCtxt) < 0) + goto cleanup; + VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(driver, qemuDomainLogContextGetManager(logCtxt), @@ -6194,6 +6202,8 @@ qemuProcessLaunch(virConnectPtr conn, ret = 0;
cleanup: + if (ret)
if (ret < 0), right?
+ qemuExtDevicesStop(driver, vm->def); qemuDomainSecretDestroy(vm); virCommandFree(cmd); virObjectUnref(logCtxt); @@ -6614,6 +6624,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainCleanupRun(driver, vm);
+ qemuExtDevicesStop(driver, vm->def); + /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm);
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c new file mode 100644 index 0000000..024d24d --- /dev/null +++ b/src/qemu/qemu_tpm.c @@ -0,0 +1,753 @@ +/* + * qemu_tpm.c: QEMU TPM support + * + * Copyright (C) 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ +
[...]
+/* + * qemuTPMCreateEmulatorStoragePath + * + * @swtpmStorageDir: directory for swtpm persistent state + * @vmname: The name of the VM for which to create the storage + * + * Create the swtpm's storage path + */ +static char * +qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, + const char *vmname)
You changed at some point to use the uuidstr - probably should change this too. Comment and argument
+{ + char *path = NULL; + + ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname)); + + return path; +} + +
[...]
+/* + * qemuTPMCreateEmulatorStorage + * + * @storagepath: directory for swtpm's pesistent state
persistent
+ * @created: a pointer to a bool that will be set to true if the + * storage was created because it did not exist yet + * @swtpm_user: The uid that needs to be able to access the directory + * @swtpm_group: The gid that needs to be able to access the directory + * + * Unless the storage path for the swtpm for the given VM + * already exists, create it and make it accessible for the given userid. + * Adapt ownership of the directory and all swtpm's state files there. + */ +static int +qemuTPMCreateEmulatorStorage(const char *storagepath, + bool *created, + uid_t swtpm_user, + gid_t swtpm_group) +{ + int ret = -1; + char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath); + + if (!swtpmStorageDir) + return -1; + + if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0) + goto cleanup; + + *created = false; + + if (!virFileExists(storagepath)) + *created = true; + + if (virDirCreate(storagepath, 0700, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not create directory %s as %u:%d"), + storagepath, swtpm_user, swtpm_group); + goto cleanup; + } + + if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(swtpmStorageDir); + + return ret; +} + +
[...]
+/* + * qemuTPMEmulatorPrepareHost: + * + * @tpm: tpm definition + * @logDir: directory where swtpm writes its logs into + * @vmname: name of the VM + * @swtpm_user: uid to run the swtpm with + * @swtpm_group: gid to run the swtpm with + * @swtpmStateDir: directory for swtpm's persistent state + * @qemu_user: uid that qemu will run with; we share the socket file with it + * @shortName: short and unique name of the domain + * + * Prepare the log directory for the swtpm and adjust ownership of it and the + * log file we will be using. Prepare the state directory where we will share + * the socket between tss and qemu users. + */ +static int +qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, + const char *logDir, + const char *vmname, + uid_t swtpm_user, + gid_t swtpm_group, + const char *swtpmStateDir, + uid_t qemu_user, + const char *shortName) +{ + int ret = -1; + + if (qemuTPMEmulatorInit() < 0) + return -1; + + /* create log dir ... */ + if (virFileMakePathWithMode(logDir, 0730) < 0) + return -1; + + /* ... and adjust ownership */ + if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup;
Do we really need both? IOW: Why not just virDirCreate
+ + /* create logfile name ... */ + if (!tpm->data.emulator.logfile && + virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log", + logDir, vmname) < 0) + goto cleanup; + + /* ... and make sure it can be accessed by swtpm_user */ + if (virFileExists(tpm->data.emulator.logfile) && + chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { + virReportSystemError(errno, + _("Could not chown on swtpm logfile %s"), + tpm->data.emulator.logfile); + goto cleanup; + } + + /* + create our swtpm state dir ... + - QEMU user needs to be able to access the socket there + - swtpm group needs to be able to create files there + - in privileged mode 0570 would be enough, for non-privileged mode + we need 0770 + */ + if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup; + + /* create the socket filename */ + if (!tpm->data.emulator.source.data.nix.path && + !(tpm->data.emulator.source.data.nix.path = + qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + goto cleanup; + tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + + ret = 0; + + cleanup: + + return ret; +}
[...]
+int +qemuExtTPMInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def)
When I see qemuExt - I generally expect the code to be in qemu_extdevice.c... Similar for anything qemuExtTPM. I understand why the Ext is there (e.g. external), but not sure it's necessary. FWIW: Although I didn't win my argument when last discussed, my feeling is static functions should be something like "tpmEmulatorInitPaths"; whereas, external functions should be qemuTPMEmulatorInitPaths. John
+{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + int ret = 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = qemuTPMEmulatorInitPaths(def->tpm, cfg->swtpmStorageDir, + def->uuid); + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + virObjectUnref(cfg); + + return ret; +} + +
[...]

On 05/15/2018 09:53 AM, John Ferlan wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/Makefile.inc.am | 4 + src/qemu/qemu_domain.c | 2 + src/qemu/qemu_driver.c | 5 + src/qemu/qemu_extdevice.c | 154 ++++++++++ src/qemu/qemu_extdevice.h | 53 ++++ src/qemu/qemu_migration.c | 3 + src/qemu/qemu_process.c | 12 + src/qemu/qemu_tpm.c | 753 ++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_tpm.h | 50 +++ 9 files changed, 1036 insertions(+) create mode 100644 src/qemu/qemu_extdevice.c create mode 100644 src/qemu/qemu_extdevice.h create mode 100644 src/qemu/qemu_tpm.c create mode 100644 src/qemu/qemu_tpm.h
[...]
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 37876b8..9370de3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -48,6 +48,7 @@ #include "qemu_migration_params.h" #include "qemu_interface.h" #include "qemu_security.h" +#include "qemu_extdevice.h"
#include "cpu/cpu.h" #include "datatypes.h" @@ -5872,6 +5873,10 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver, if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0) goto cleanup;
+ VIR_DEBUG("Preparing external devices"); + if (qemuExtDevicesPrepareHost(driver, vm->def) < 0) + goto cleanup; + ret = 0; cleanup: virObjectUnref(cfg); @@ -5955,6 +5960,9 @@ qemuProcessLaunch(virConnectPtr conn, goto cleanup; logfile = qemuDomainLogContextGetWriteFD(logCtxt);
+ if (qemuExtDevicesStart(driver, vm->def, logCtxt) < 0) + goto cleanup; + VIR_DEBUG("Building emulator command line"); if (!(cmd = qemuBuildCommandLine(driver, qemuDomainLogContextGetManager(logCtxt), @@ -6194,6 +6202,8 @@ qemuProcessLaunch(virConnectPtr conn, ret = 0;
cleanup: + if (ret) if (ret < 0), right?
Fixed.
+ qemuExtDevicesStop(driver, vm->def); qemuDomainSecretDestroy(vm); virCommandFree(cmd); virObjectUnref(logCtxt); @@ -6614,6 +6624,8 @@ void qemuProcessStop(virQEMUDriverPtr driver,
qemuDomainCleanupRun(driver, vm);
+ qemuExtDevicesStop(driver, vm->def); + /* Stop autodestroy in case guest is restarted */ qemuProcessAutoDestroyRemove(driver, vm);
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c new file mode 100644 index 0000000..024d24d --- /dev/null +++ b/src/qemu/qemu_tpm.c @@ -0,0 +1,753 @@ +/* + * qemu_tpm.c: QEMU TPM support + * + * Copyright (C) 2018 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/>. + * + * Author: Stefan Berger <stefanb@linux.vnet.ibm.com> + */ + [...]
+/* + * qemuTPMCreateEmulatorStoragePath + * + * @swtpmStorageDir: directory for swtpm persistent state + * @vmname: The name of the VM for which to create the storage + * + * Create the swtpm's storage path + */ +static char * +qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, + const char *vmname) You changed at some point to use the uuidstr - probably should change this too. Comment and argument
Fixed.
+{ + char *path = NULL; + + ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname)); + + return path; +} + + [...]
+/* + * qemuTPMCreateEmulatorStorage + * + * @storagepath: directory for swtpm's pesistent state persistent
Fixed.
+ * @created: a pointer to a bool that will be set to true if the + * storage was created because it did not exist yet + * @swtpm_user: The uid that needs to be able to access the directory + * @swtpm_group: The gid that needs to be able to access the directory + * + * Unless the storage path for the swtpm for the given VM + * already exists, create it and make it accessible for the given userid. + * Adapt ownership of the directory and all swtpm's state files there. + */ +static int +qemuTPMCreateEmulatorStorage(const char *storagepath, + bool *created, + uid_t swtpm_user, + gid_t swtpm_group) +{ + int ret = -1; + char *swtpmStorageDir = qemuTPMGetTPMStorageDir(storagepath); + + if (!swtpmStorageDir) + return -1; + + if (qemuTPMEmulatorInitStorage(swtpmStorageDir) < 0) + goto cleanup; + + *created = false; + + if (!virFileExists(storagepath)) + *created = true; + + if (virDirCreate(storagepath, 0700, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not create directory %s as %u:%d"), + storagepath, swtpm_user, swtpm_group); + goto cleanup; + } + + if (virFileChownFiles(storagepath, swtpm_user, swtpm_group) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(swtpmStorageDir); + + return ret; +} + + [...]
+/* + * qemuTPMEmulatorPrepareHost: + * + * @tpm: tpm definition + * @logDir: directory where swtpm writes its logs into + * @vmname: name of the VM + * @swtpm_user: uid to run the swtpm with + * @swtpm_group: gid to run the swtpm with + * @swtpmStateDir: directory for swtpm's persistent state + * @qemu_user: uid that qemu will run with; we share the socket file with it + * @shortName: short and unique name of the domain + * + * Prepare the log directory for the swtpm and adjust ownership of it and the + * log file we will be using. Prepare the state directory where we will share + * the socket between tss and qemu users. + */ +static int +qemuTPMEmulatorPrepareHost(virDomainTPMDefPtr tpm, + const char *logDir, + const char *vmname, + uid_t swtpm_user, + gid_t swtpm_group, + const char *swtpmStateDir, + uid_t qemu_user, + const char *shortName) +{ + int ret = -1; + + if (qemuTPMEmulatorInit() < 0) + return -1; + + /* create log dir ... */ + if (virFileMakePathWithMode(logDir, 0730) < 0) + return -1; + + /* ... and adjust ownership */ + if (virDirCreate(logDir, 0730, swtpm_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup; Do we really need both? IOW: Why not just virDirCreate
virFileMakePathWithMode creates all missing directories before. What is wrong about that call is that it needs to have permissions 0711. If we don't call it we'll get an error because /var/log/swtpm and /var/log/swtpm/libvirt are missing. error: failed to create directory '/var/log/swtpm/libvirt/qemu': No such file or directory The rpm does this correctly.
+ + /* create logfile name ... */ + if (!tpm->data.emulator.logfile && + virAsprintf(&tpm->data.emulator.logfile, "%s/%s-swtpm.log", + logDir, vmname) < 0) + goto cleanup; + + /* ... and make sure it can be accessed by swtpm_user */ + if (virFileExists(tpm->data.emulator.logfile) && + chown(tpm->data.emulator.logfile, swtpm_user, swtpm_group) < 0) { + virReportSystemError(errno, + _("Could not chown on swtpm logfile %s"), + tpm->data.emulator.logfile); + goto cleanup; + } + + /* + create our swtpm state dir ... + - QEMU user needs to be able to access the socket there + - swtpm group needs to be able to create files there + - in privileged mode 0570 would be enough, for non-privileged mode + we need 0770 + */ + if (virDirCreate(swtpmStateDir, 0770, qemu_user, swtpm_group, + VIR_DIR_CREATE_ALLOW_EXIST) < 0) + goto cleanup; + + /* create the socket filename */ + if (!tpm->data.emulator.source.data.nix.path && + !(tpm->data.emulator.source.data.nix.path = + qemuTPMCreateEmulatorSocket(swtpmStateDir, shortName))) + goto cleanup; + tpm->data.emulator.source.type = VIR_DOMAIN_CHR_TYPE_UNIX; + + ret = 0; + + cleanup: + + return ret; +} [...]
+int +qemuExtTPMInitPaths(virQEMUDriverPtr driver, + virDomainDefPtr def) When I see qemuExt - I generally expect the code to be in qemu_extdevice.c... Similar for anything qemuExtTPM. I understand why the Ext is there (e.g. external), but not sure it's necessary.
There's some more TPM specific code in the qemuExtTPM* functions and I thought I'd rather not clutter up the qemu_extdevice.c with the TPM specifics but keep it at def->tpm checks and push the rest into qemu_tpm.c.
FWIW: Although I didn't win my argument when last discussed, my feeling is static functions should be something like "tpmEmulatorInitPaths"; whereas, external functions should be qemuTPMEmulatorInitPaths.
I am following existing pattern when prefixing all functions with 'qemu' for code in src/qemu/qemu_*.c. qemu_driver.c seems to do the same. Stefan

On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> ---
[…snip…]
+ +/* + * qemuExtTPMStartEmulator: + * + * @driver: QEMU driver + * @def: domain definition + * @logCtxt: log context + * + * Start the external TPM Emulator: + * - have the command line built + * - start the external TPM Emulator and sync with it before QEMU start + */ +static int +qemuExtTPMStartEmulator(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) +{ + int ret = -1; + virCommandPtr cmd = NULL; + int exitstatus; + char *errbuf = NULL; + virQEMUDriverConfigPtr cfg; + virDomainTPMDefPtr tpm = def->tpm; + char *shortName = virDomainDefGetShortName(def); + + if (!shortName) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + /* stop any left-over TPM emulator for this VM */ + qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + + if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid, + driver->privileged, + cfg->swtpm_user, + cfg->swtpm_group))) + goto cleanup; + + if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) + goto cleanup; + + virCommandSetErrorBuffer(cmd, &errbuf); + + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " + "stderr: %s"), exitstatus, errbuf); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'swtpm'. exitstatus: %d, " + "error: %s"), exitstatus, errbuf);
Do we need both? (VIR_ERROR and virReportError)? […snip] Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 11:25 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
Implement functions for managing the storage of the external swtpm as well as starting and stopping it. Also implement functions to use swtpm_setup, which simulates the manufacturing of a TPM, which includes creation of certificates for the device.
Further, the external TPM needs storage on the host that we need to set up before it can be run. We can clean up the host once the domain is undefined.
This patch also implements a small layer for external device support that calls into the TPM device layer if a domain has an attached TPM. This is the layer we will wire up later on.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- […snip…]
+ +/* + * qemuExtTPMStartEmulator: + * + * @driver: QEMU driver + * @def: domain definition + * @logCtxt: log context + * + * Start the external TPM Emulator: + * - have the command line built + * - start the external TPM Emulator and sync with it before QEMU start + */ +static int +qemuExtTPMStartEmulator(virQEMUDriverPtr driver, + virDomainDefPtr def, + qemuDomainLogContextPtr logCtxt) +{ + int ret = -1; + virCommandPtr cmd = NULL; + int exitstatus; + char *errbuf = NULL; + virQEMUDriverConfigPtr cfg; + virDomainTPMDefPtr tpm = def->tpm; + char *shortName = virDomainDefGetShortName(def); + + if (!shortName) + return -1; + + cfg = virQEMUDriverGetConfig(driver); + + /* stop any left-over TPM emulator for this VM */ + qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + + if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid, + driver->privileged, + cfg->swtpm_user, + cfg->swtpm_group))) + goto cleanup; + + if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) + goto cleanup; + + virCommandSetErrorBuffer(cmd, &errbuf); + + if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " + "stderr: %s"), exitstatus, errbuf); + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Could not start 'swtpm'. exitstatus: %d, " + "error: %s"), exitstatus, errbuf); Do we need both? (VIR_ERROR and virReportError)?
Removed the former. Stefan

This patch adds support for an external swtpm TPM emulator. The XML for this type of TPM looks as follows: <tpm model='tpm-tis'> <backend type='emulator'/> </tpm> The XML will currently only start a TPM 1.2. Upon first start, libvirt will run `swtpm_setup`, which will simulate the manufacturing of a TPM and create certificates for it and write them into NVRAM locations of the emulated TPM. After that libvirt starts the swtpm TPM emulator using the `swtpm` executable. Once the VM terminates, libvirt uses the swtpm_ioctl executable to gracefully shut down the `swtpm` in case it is still running (QEMU did not send shutdown) or clean up the socket file. The above mentioned executables must be found in the PATH. The executables can either be run as root or started as root and switch to the tss user. The requirement for the tss user comes through 'tcsd', which is used for the simulation of the manufacturing. Which user is used can be configured through qemu.conf. By default 'tss' is used. The swtpm writes out state into files. The state is kept in /var/lib/libvirt/swtpm: [root@localhost libvirt]# ls -lZ | grep swtpm drwx--x--x. 7 root root unconfined_u:object_r:virt_var_lib_t:s0 4096 Apr 5 16:22 swtpm The directory /var/lib/libvirt/swtpm maintains per-TPM state directories. (Using the uuid of the VM for that since the name can change per VM renaming but we need a stable directory name.) [root@localhost swtpm]# ls -lZ total 4 drwx------. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 5 16:46 485d0004-a48f-436a-8457-8a3b73e28568 [root@localhost 485d0004-a48f-436a-8457-8a3b73e28568]# ls -lZ total 4 drwx------. 2 tss tss system_u:object_r:virt_var_lib_t:s0 4096 Apr 10 21:34 tpm1.2 [root@localhost tpm1.2]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:virt_var_lib_t:s0 3648 Apr 5 16:46 tpm-00.permall The directory /var/run/libvirt/qemu/swtpm/ hosts the swtpm.sock that QEMU uses to communicate with the swtpm: root@localhost domain-1-testvm]# ls -lZ total 0 srw-------. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 6 10:24 1-testvm-swtpm.sock The logfile for the swtpm is in /var/log/swtpm/libvirt/qemu: [root@localhost-3 qemu]# ls -lZ total 4 -rw-------. 1 tss tss unconfined_u:object_r:var_log_t:s0 2199 Apr 6 14:01 testvm-swtpm.log The processes are labeled as follows: [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep socket | grep -v grep system_u:system_r:virtd_t:s0-s0:c0.c1023 tss 18697 0.0 0.0 28172 3892 ? Ss 16:46 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c413,c430 qemu 18702 2.5 0.0 3036052 48676 ? Sl 16:46 0:08 /bin/qemu-system-x86_64 [...] Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> Reviewed-by: John Ferlan <jferlan@redhat.com> --- src/qemu/qemu_command.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 151f4fc..0e3ef99 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -9361,17 +9361,27 @@ qemuBuildTPMBackendStr(const virDomainDef *def, virCommandPtr cmd, virQEMUCapsPtr qemuCaps, int *tpmfd, - int *cancelfd) + int *cancelfd, + char **chardev) { const virDomainTPMDef *tpm = def->tpm; virBuffer buf = VIR_BUFFER_INITIALIZER; - const char *type = virDomainTPMBackendTypeToString(tpm->type); + const char *type = NULL; char *cancel_path = NULL, *devset = NULL; const char *tpmdev; *tpmfd = -1; *cancelfd = -1; + switch (tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + type = virDomainTPMBackendTypeToString(tpm->type); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + goto error; + } + virBufferAsprintf(&buf, "%s,id=tpm-%s", type, tpm->info.alias); switch (tpm->type) { @@ -9423,6 +9433,16 @@ qemuBuildTPMBackendStr(const virDomainDef *def, break; case VIR_DOMAIN_TPM_TYPE_EMULATOR: + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_TPM_EMULATOR)) + goto no_support; + + virBufferAddLit(&buf, ",chardev=chrtpm"); + + if (virAsprintf(chardev, "socket,id=chrtpm,path=%s", + tpm->data.emulator.source.data.nix.path) < 0) + goto error; + + break; case VIR_DOMAIN_TPM_TYPE_LAST: goto error; } @@ -9453,6 +9473,7 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, virQEMUCapsPtr qemuCaps) { char *optstr; + char *chardev = NULL; int tpmfd = -1; int cancelfd = -1; char *fdset; @@ -9461,12 +9482,18 @@ qemuBuildTPMCommandLine(virCommandPtr cmd, return 0; if (!(optstr = qemuBuildTPMBackendStr(def, cmd, qemuCaps, - &tpmfd, &cancelfd))) + &tpmfd, &cancelfd, + &chardev))) return -1; virCommandAddArgList(cmd, "-tpmdev", optstr, NULL); VIR_FREE(optstr); + if (chardev) { + virCommandAddArgList(cmd, "-chardev", chardev, NULL); + VIR_FREE(chardev); + } + if (tpmfd >= 0) { fdset = qemuVirCommandGetFDSet(cmd, tpmfd); if (!fdset) -- 2.5.5

This patch adds extensions to existing test cases and specific test cases for the tpm-emulator. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- .../tpm-emulator.x86_64-latest.args | 33 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 15 +++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args diff --git a/tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args new file mode 100644 index 0000000..82b676f --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +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 \ +-realtime mlock=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,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,\ +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 ddf567b..26f9d70 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -532,6 +532,19 @@ testCompareXMLToArgv(const void *data) } } + if (vm->def->tpm) { + switch (vm->def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + if (VIR_STRDUP(vm->def->tpm->data.emulator.source.data.file.path, + "/dev/test") < 0) + goto cleanup; + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + } + if (!(cmd = qemuProcessCreatePretendCmd(&driver, vm, migrateURI, (flags & FLAG_FIPS), false, VIR_QEMU_PROCESS_START_COLD))) { @@ -1989,7 +2002,7 @@ mymain(void) QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_CRB); DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); - + DO_TEST_CAPS_LATEST("tpm-emulator"); DO_TEST_PARSE_ERROR("pci-domain-invalid", NONE); DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE); -- 2.5.5

On 05/10/2018 05:57 PM, Stefan Berger wrote:
This patch adds extensions to existing test cases and specific test cases for the tpm-emulator.
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- .../tpm-emulator.x86_64-latest.args | 33 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 15 +++++++++- 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator.x86_64-latest.args
Reviewed-by: John Ferlan <jferlan@redhat.com> John

In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated. The file and process labels now look as follows: Directory: /var/lib/libvirt/swtpm [root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm [root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall The log in /var/log/swtpm/libvirt/qemu is labeled as follows: -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log [root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..] Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virCommandSetErrorBuffer(cmd, &errbuf); - if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, @@ -696,6 +715,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, ret = 0; cleanup: + if (ret < 0) + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); VIR_FREE(shortName); VIR_FREE(errbuf); virCommandFree(cmd); @@ -741,6 +762,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver, goto cleanup; qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: case VIR_DOMAIN_TPM_TYPE_LAST: diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 95e7c4d..cbf0ecf 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -149,6 +149,10 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd); +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def); +typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def); struct _virSecurityDriver { @@ -213,6 +217,9 @@ struct _virSecurityDriver { virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel; virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel; + + virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels; + virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels; }; virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 71f7f59..8683ad7 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1204,3 +1204,39 @@ virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virReportUnsupportedError(); return -1; } + + +int +virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainSetSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} + + +int +virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainRestoreSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index c36a8b4..e772b61 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -194,4 +194,10 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd); +int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + +int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 92e8415..6377fb7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3048,6 +3048,167 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr, return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel); } + +/* + * _virSecuritySELinuxSetFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * @seclabel: the security label + * + * Set the file labels on the given path; if the path is a directory + * we label all files found there, including the directory itself, + * otherwise we just label the file. + */ +static int +_virSecuritySELinuxSetFileLabels(virSecurityManagerPtr mgr, + const char *path, + virSecurityLabelDefPtr seclabel) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxSetFilecon(mgr, filename, + seclabel->imagelabel); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to label files under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +/* + * _virSecuritySELinuxRestoreFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * + * Restore the file labels on the given path; if the path is a directory + * we restore all file labels found there, including the label of the + * directory itself, otherwise we just restore the label on the file. + */ +static int +_virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr, + const char *path) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxRestoreFileLabel(mgr, filename); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to restore file labels under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +static int +virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + virSecurityLabelDefPtr seclabel; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.storagepath, + seclabel); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.logfile, + seclabel); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +static int +virSecuritySELinuxRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.storagepath); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.logfile); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + virSecurityDriver virSecurityDriverSELinux = { .privateDataLen = sizeof(virSecuritySELinuxData), .name = SECURITY_SELINUX_NAME, @@ -3107,4 +3268,7 @@ virSecurityDriver virSecurityDriverSELinux = { .domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecuritySELinuxSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecuritySELinuxRestoreTPMLabels, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 9615f9f..e37a681 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -760,6 +760,43 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr, return rc; } + +static int +virSecurityStackSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + +static int +virSecurityStackRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -822,4 +859,7 @@ virSecurityDriver virSecurityDriverStack = { .domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels, }; -- 2.5.5

On 05/10/2018 11:57 PM, Stefan Berger wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels;
Shouldn't there be wrappers for virSecurityManagerRestoreTPMLabels virSecurityManagerSetTPMLabels in src/qemu/qemu_security.h and possibly src/qemu/qemu_security.c?
virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c make syntax-check error
0.03 prohibit_virConnectOpen_in_virsh prohibit_virSecurity ../src/qemu/qemu_tpm.c:812: if (virSecurityManagerSetTPMLabels(driver->securityManager, ../src/qemu/qemu_tpm.c:816: if (virSecurityManagerSetChildProcessLabel(driver->securityManager, ../src/qemu/qemu_tpm.c:820: if (virSecurityManagerPreFork(driver->securityManager) < 0) ../src/qemu/qemu_tpm.c:829: virSecurityManagerPostFork(driver->securityManager); ../src/qemu/qemu_tpm.c:860: virSecurityManagerRestoreTPMLabels(driver->securityManager, def); ../src/qemu/qemu_tpm.c:911: virSecurityManagerRestoreTPMLabels(driver->securityManager, def); maint.mk: prefer qemuSecurity wrappers ../cfg.mk:998: recipe for target 'sc_prohibit_virSecurity' failed make: *** [sc_prohibit_virSecurity] Error 1
@@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, @@ -696,6 +715,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, ret = 0;
cleanup: + if (ret < 0) + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); VIR_FREE(shortName); VIR_FREE(errbuf); virCommandFree(cmd); @@ -741,6 +762,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver, goto cleanup;
qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: case VIR_DOMAIN_TPM_TYPE_LAST: diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 95e7c4d..cbf0ecf 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -149,6 +149,10 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd); +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def); +typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def);
struct _virSecurityDriver { @@ -213,6 +217,9 @@ struct _virSecurityDriver {
virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel; virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel; + + virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels; + virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels; };
virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 71f7f59..8683ad7 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1204,3 +1204,39 @@ virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virReportUnsupportedError(); return -1; } + + +int +virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainSetSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} + + +int +virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainRestoreSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index c36a8b4..e772b61 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -194,4 +194,10 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd);
+int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + +int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 92e8415..6377fb7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3048,6 +3048,167 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr, return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel); }
+ +/* + * _virSecuritySELinuxSetFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * @seclabel: the security label + * + * Set the file labels on the given path; if the path is a directory + * we label all files found there, including the directory itself, + * otherwise we just label the file. + */ +static int +_virSecuritySELinuxSetFileLabels(virSecurityManagerPtr mgr, + const char *path, + virSecurityLabelDefPtr seclabel) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxSetFilecon(mgr, filename, + seclabel->imagelabel); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to label files under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +/* + * _virSecuritySELinuxRestoreFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * + * Restore the file labels on the given path; if the path is a directory + * we restore all file labels found there, including the label of the + * directory itself, otherwise we just restore the label on the file. + */ +static int +_virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr, + const char *path) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxRestoreFileLabel(mgr, filename); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to restore file labels under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +static int +virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + virSecurityLabelDefPtr seclabel; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.storagepath, + seclabel); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.logfile, + seclabel); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +static int +virSecuritySELinuxRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.storagepath); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.logfile); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + virSecurityDriver virSecurityDriverSELinux = { .privateDataLen = sizeof(virSecuritySELinuxData), .name = SECURITY_SELINUX_NAME, @@ -3107,4 +3268,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecuritySELinuxSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecuritySELinuxRestoreTPMLabels, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 9615f9f..e37a681 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -760,6 +760,43 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr, return rc; }
+ +static int +virSecurityStackSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + +static int +virSecurityStackRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -822,4 +859,7 @@ virSecurityDriver virSecurityDriverStack = {
.domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels, };
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 06:30 AM, Boris Fiuczynski wrote:
On 05/10/2018 11:57 PM, Stefan Berger wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels;
Shouldn't there be wrappers for virSecurityManagerRestoreTPMLabels virSecurityManagerSetTPMLabels in src/qemu/qemu_security.h and possibly src/qemu/qemu_security.c?
virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c make syntax-check error
0.03 prohibit_virConnectOpen_in_virsh prohibit_virSecurity ../src/qemu/qemu_tpm.c:812: if (virSecurityManagerSetTPMLabels(driver->securityManager, ../src/qemu/qemu_tpm.c:816: if (virSecurityManagerSetChildProcessLabel(driver->securityManager, ../src/qemu/qemu_tpm.c:820: if (virSecurityManagerPreFork(driver->securityManager) < 0) ../src/qemu/qemu_tpm.c:829: virSecurityManagerPostFork(driver->securityManager); ../src/qemu/qemu_tpm.c:860: virSecurityManagerRestoreTPMLabels(driver->securityManager, def); ../src/qemu/qemu_tpm.c:911: virSecurityManagerRestoreTPMLabels(driver->securityManager, def); maint.mk: prefer qemuSecurity wrappers ../cfg.mk:998: recipe for target 'sc_prohibit_virSecurity' failed make: *** [sc_prohibit_virSecurity] Error 1
I wrapped this now in two functions: int qemuSecurityStartTPMEmulator(virQEMUDriverPtr driver, virDomainDefPtr def, virCommandPtr cmd, uid_t uid, gid_t gid, int *exitstatus, int *cmdret); void qemuSecurityCleanupTPMEmulator(virQEMUDriverPtr driver, virDomainDefPtr def); I will repost a v5 later today. Stefan

On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR,
Don't we have to set ret to -1 here (for the case where ret == 0 && exitstatus != 0)? […snip] Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 11:38 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, Don't we have to set ret to -1 here (for the case where ret == 0 && exitstatus != 0)?
Very true. Stefan

On 05/15/2018 11:45 AM, Stefan Berger wrote:
On 05/15/2018 11:38 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, Don't we have to set ret to -1 here (for the case where ret == 0 && exitstatus != 0)?
Very true.
ret is initialized with '-1'. So we are good. Stefan

On Tue, May 15, 2018 at 05:50 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
On 05/15/2018 11:45 AM, Stefan Berger wrote:
On 05/15/2018 11:38 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, Don't we have to set ret to -1 here (for the case where ret == 0 && exitstatus != 0)?
Very true.
ret is initialized with '-1'. So we are good.
No we are not good since @ret is overwritten with: ret = virCommandRun(cmd, &exitstatus); So it’s possible that virCommandRun returns 0 but exitstatus != 0 => ret == 0; exitstatus != 0 => virReportError(…) reports an error but the return value @ret remains 0.
Stefan
Beste Grüße / Kind regards Marc Hartmayer IBM Deutschland Research & Development GmbH Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/15/2018 12:13 PM, Marc Hartmayer wrote:
On Tue, May 15, 2018 at 05:50 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
On 05/15/2018 11:45 AM, Stefan Berger wrote:
On 05/15/2018 11:38 AM, Marc Hartmayer wrote:
On Thu, May 10, 2018 at 11:57 PM +0200, Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, Don't we have to set ret to -1 here (for the case where ret == 0 && exitstatus != 0)? Very true. ret is initialized with '-1'. So we are good. No we are not good since @ret is overwritten with:
ret = virCommandRun(cmd, &exitstatus);
So it’s possible that virCommandRun returns 0 but exitstatus != 0 => ret == 0; exitstatus != 0 => virReportError(…) reports an error but the return value @ret remains 0.
After the modifications the patch looks like this and doesn't touch 'ret' anymore: @@ -684,7 +686,12 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virCommandSetErrorBuffer(cmd, &errbuf); - if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (qemuSecurityStartTPMEmulator(driver, def, cmd, + cfg->swtpm_user, cfg->swtpm_group, + &exitstatus, &cmdret) < 0) + goto cleanup; + + if (cmdret < 0 || exitstatus != 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not start 'swtpm'. exitstatus: %d, " "error: %s"), exitstatus, errbuf); Stefan

On 05/10/2018 05:57 PM, Stefan Berger wrote:
In this patch we label the swtpm process with SELinux labels. We give it the same label as the QEMU process has. We label its state directory and files as well. We restore the old security labels once the swtpm has terminated.
The file and process labels now look as follows:
Directory: /var/lib/libvirt/swtpm
[root@localhost swtpm]# ls -lZ total 4 rwx------. 2 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 4096 Apr 5 16:46 testvm
[root@localhost testvm]# ls -lZ total 8 -rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 3648 Apr 5 16:46 tpm-00.permall
The log in /var/log/swtpm/libvirt/qemu is labeled as follows:
-rw-r--r--. 1 tss tss system_u:object_r:svirt_image_t:s0:c254,c932 2237 Apr 5 16:46 vtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep swtpm | grep ctrl | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 tss 25664 0.0 0.0 28172 3892 ? Ss 16:57 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm1.2 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log
[root@localhost 485d0004-a48f-436a-8457-8a3b73e28567]# ps auxZ | grep qemu | grep tpm | grep -v grep system_u:system_r:svirt_t:s0:c254,c932 qemu 25669 99.0 0.0 3096704 48500 ? Sl 16:57 3:28 /bin/qemu-system-x86_64 [..]
An outstanding issue with this patch are the additional svirt rules that are needed. A challenge here is to find that subset of rules that are really needed. One of my machines with Fedora 27 needs only this one rule (following permissive mode and audit2allow): allow svirt_t swtpm_exec_t:file map; The other FC27 machine needs these: allow svirt_t bin_t:file entrypoint; allow svirt_t swtpm_exec_t:file { entrypoint execute map read }; I have seen it needing several more rules than these. I suppose it won't be a problem to get them accepted, assuming there will be an swtpm_exec_t... Stefan
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/libvirt_private.syms | 2 + src/qemu/qemu_tpm.c | 24 +++++- src/security/security_driver.h | 7 ++ src/security/security_manager.c | 36 +++++++++ src/security/security_manager.h | 6 ++ src/security/security_selinux.c | 164 ++++++++++++++++++++++++++++++++++++++++ src/security/security_stack.c | 40 ++++++++++ 7 files changed, 278 insertions(+), 1 deletion(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 75b8932..2ce67e7 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1319,6 +1319,7 @@ virSecurityManagerRestoreImageLabel; virSecurityManagerRestoreInputLabel; virSecurityManagerRestoreMemoryLabel; virSecurityManagerRestoreSavedStateLabel; +virSecurityManagerRestoreTPMLabels; virSecurityManagerSetAllLabel; virSecurityManagerSetChardevLabel; virSecurityManagerSetChildProcessLabel; @@ -1333,6 +1334,7 @@ virSecurityManagerSetProcessLabel; virSecurityManagerSetSavedStateLabel; virSecurityManagerSetSocketLabel; virSecurityManagerSetTapFDLabel; +virSecurityManagerSetTPMLabels; virSecurityManagerStackAddNested; virSecurityManagerTransactionAbort; virSecurityManagerTransactionCommit; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 024d24d..62f0146 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -684,7 +684,26 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver,
virCommandSetErrorBuffer(cmd, &errbuf);
- if (virCommandRun(cmd, &exitstatus) < 0 || exitstatus != 0) { + if (virSecurityManagerSetTPMLabels(driver->securityManager, + def) < 0) + goto cleanup; + + if (virSecurityManagerSetChildProcessLabel(driver->securityManager, + def, cmd) < 0) + goto cleanup; + + if (virSecurityManagerPreFork(driver->securityManager) < 0) + goto cleanup; + + /* make sure we run this with the appropriate user */ + virCommandSetUID(cmd, cfg->swtpm_user); + virCommandSetGID(cmd, cfg->swtpm_group); + + ret = virCommandRun(cmd, &exitstatus); + + virSecurityManagerPostFork(driver->securityManager); + + if (ret < 0 || exitstatus != 0) { VIR_ERROR(_("Could not start 'swtpm'. exitstatus: %d " "stderr: %s"), exitstatus, errbuf); virReportError(VIR_ERR_INTERNAL_ERROR, @@ -696,6 +715,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, ret = 0;
cleanup: + if (ret < 0) + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); VIR_FREE(shortName); VIR_FREE(errbuf); virCommandFree(cmd); @@ -741,6 +762,7 @@ qemuExtTPMStop(virQEMUDriverPtr driver, goto cleanup;
qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName); + virSecurityManagerRestoreTPMLabels(driver->securityManager, def); break; case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: case VIR_DOMAIN_TPM_TYPE_LAST: diff --git a/src/security/security_driver.h b/src/security/security_driver.h index 95e7c4d..cbf0ecf 100644 --- a/src/security/security_driver.h +++ b/src/security/security_driver.h @@ -149,6 +149,10 @@ typedef int (*virSecurityDomainRestoreChardevLabel) (virSecurityManagerPtr mgr, virDomainDefPtr def, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd); +typedef int (*virSecurityDomainSetTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def); +typedef int (*virSecurityDomainRestoreTPMLabels) (virSecurityManagerPtr mgr, + virDomainDefPtr def);
struct _virSecurityDriver { @@ -213,6 +217,9 @@ struct _virSecurityDriver {
virSecurityDomainSetChardevLabel domainSetSecurityChardevLabel; virSecurityDomainRestoreChardevLabel domainRestoreSecurityChardevLabel; + + virSecurityDomainSetTPMLabels domainSetSecurityTPMLabels; + virSecurityDomainRestoreTPMLabels domainRestoreSecurityTPMLabels; };
virSecurityDriverPtr virSecurityDriverLookup(const char *name, diff --git a/src/security/security_manager.c b/src/security/security_manager.c index 71f7f59..8683ad7 100644 --- a/src/security/security_manager.c +++ b/src/security/security_manager.c @@ -1204,3 +1204,39 @@ virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virReportUnsupportedError(); return -1; } + + +int +virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainSetSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainSetSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} + + +int +virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + int ret; + + if (mgr->drv->domainRestoreSecurityTPMLabels) { + virObjectLock(mgr); + ret = mgr->drv->domainRestoreSecurityTPMLabels(mgr, vm); + virObjectUnlock(mgr); + + return ret; + } + + return 0; +} diff --git a/src/security/security_manager.h b/src/security/security_manager.h index c36a8b4..e772b61 100644 --- a/src/security/security_manager.h +++ b/src/security/security_manager.h @@ -194,4 +194,10 @@ int virSecurityManagerRestoreChardevLabel(virSecurityManagerPtr mgr, virDomainChrSourceDefPtr dev_source, bool chardevStdioLogd);
+int virSecurityManagerSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + +int virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm); + #endif /* VIR_SECURITY_MANAGER_H__ */ diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index 92e8415..6377fb7 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3048,6 +3048,167 @@ virSecuritySELinuxDomainSetPathLabel(virSecurityManagerPtr mgr, return virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel); }
+ +/* + * _virSecuritySELinuxSetFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * @seclabel: the security label + * + * Set the file labels on the given path; if the path is a directory + * we label all files found there, including the directory itself, + * otherwise we just label the file. + */ +static int +_virSecuritySELinuxSetFileLabels(virSecurityManagerPtr mgr, + const char *path, + virSecurityLabelDefPtr seclabel) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxSetFilecon(mgr, path, seclabel->imagelabel))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxSetFilecon(mgr, filename, + seclabel->imagelabel); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to label files under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +/* + * _virSecuritySELinuxRestoreFileLabels: + * + * @mgr: the virSecurityManager + * @path: path to a directory or a file + * + * Restore the file labels on the given path; if the path is a directory + * we restore all file labels found there, including the label of the + * directory itself, otherwise we just restore the label on the file. + */ +static int +_virSecuritySELinuxRestoreFileLabels(virSecurityManagerPtr mgr, + const char *path) +{ + int ret = 0; + struct dirent *ent; + char *filename = NULL; + DIR *dir; + + if ((ret = virSecuritySELinuxRestoreFileLabel(mgr, path))) + return ret; + + if (!virFileIsDir(path)) + return 0; + + if (virDirOpen(&dir, path) < 0) + return -1; + + while ((ret = virDirRead(dir, &ent, path)) > 0) { + if (ent->d_type != DT_REG) + continue; + + if (virAsprintf(&filename, "%s/%s", path, ent->d_name) < 0) { + ret = -1; + break; + } + ret = virSecuritySELinuxRestoreFileLabel(mgr, filename); + VIR_FREE(filename); + if (ret < 0) + break; + } + if (ret < 0) + virReportSystemError(errno, _("Unable to restore file labels under %s"), + path); + + virDirClose(&dir); + + return ret; +} + + +static int +virSecuritySELinuxSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + virSecurityLabelDefPtr seclabel; + + seclabel = virDomainDefGetSecurityLabelDef(def, SECURITY_SELINUX_NAME); + if (seclabel == NULL) + return 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.storagepath, + seclabel); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxSetFileLabels( + mgr, def->tpm->data.emulator.logfile, + seclabel); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + +static int +virSecuritySELinuxRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr def) +{ + int ret = 0; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + break; + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.storagepath); + if (ret == 0 && def->tpm->data.emulator.logfile) + ret = _virSecuritySELinuxRestoreFileLabels( + mgr, def->tpm->data.emulator.logfile); + break; + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + return ret; +} + + virSecurityDriver virSecurityDriverSELinux = { .privateDataLen = sizeof(virSecuritySELinuxData), .name = SECURITY_SELINUX_NAME, @@ -3107,4 +3268,7 @@ virSecurityDriver virSecurityDriverSELinux = {
.domainSetSecurityChardevLabel = virSecuritySELinuxSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecuritySELinuxRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecuritySELinuxSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecuritySELinuxRestoreTPMLabels, }; diff --git a/src/security/security_stack.c b/src/security/security_stack.c index 9615f9f..e37a681 100644 --- a/src/security/security_stack.c +++ b/src/security/security_stack.c @@ -760,6 +760,43 @@ virSecurityStackDomainRestoreChardevLabel(virSecurityManagerPtr mgr, return rc; }
+ +static int +virSecurityStackSetTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerSetTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + +static int +virSecurityStackRestoreTPMLabels(virSecurityManagerPtr mgr, + virDomainDefPtr vm) +{ + virSecurityStackDataPtr priv = virSecurityManagerGetPrivateData(mgr); + virSecurityStackItemPtr item = priv->itemsHead; + int rc = 0; + + for (; item; item = item->next) { + if (virSecurityManagerRestoreTPMLabels(item->securityManager, + vm) < 0) + rc = -1; + } + + return rc; +} + + virSecurityDriver virSecurityDriverStack = { .privateDataLen = sizeof(virSecurityStackData), .name = "stack", @@ -822,4 +859,7 @@ virSecurityDriver virSecurityDriverStack = {
.domainSetSecurityChardevLabel = virSecurityStackDomainSetChardevLabel, .domainRestoreSecurityChardevLabel = virSecurityStackDomainRestoreChardevLabel, + + .domainSetSecurityTPMLabels = virSecurityStackSetTPMLabels, + .domainRestoreSecurityTPMLabels = virSecurityStackRestoreTPMLabels, };

This patch extends the TPM's device XML with TPM 2 support. This only works for the emulator type backend and looks as follows: <tpm model='tpm-tis'> <backend type='emulator' version='2'/> </tpm> The swtpm process now has --tpm2 as an additional parameter: system_u:system_r:svirt_t:s0:c597,c632 tss 18477 11.8 0.0 28364 3868 ? Rs 11:13 13:50 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/testvm-swtpm.sock,mode=0660 --tpmstate dir=/var/lib/libvirt/swtpm/testvm/tpm2,mode=0640 --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --tpm2 --pid file=/var/run/libvirt/qemu/swtpm/testvm-swtpm.pid The version of the TPM can be changed and the state of the TPM is preserved. Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- docs/formatdomain.html.in | 15 +++++- docs/schemas/domaincommon.rng | 12 +++++ src/conf/domain_conf.c | 27 +++++++++- src/conf/domain_conf.h | 6 +++ src/qemu/qemu_tpm.c | 63 ++++++++++++++++++++-- .../tpm-emulator-tpm2.x86_64-latest.args | 33 ++++++++++++ tests/qemuxml2argvdata/tpm-emulator-tpm2.xml | 30 +++++++++++ tests/qemuxml2argvtest.c | 1 + tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml | 34 ++++++++++++ 9 files changed, 216 insertions(+), 5 deletions(-) create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args create mode 100644 tests/qemuxml2argvdata/tpm-emulator-tpm2.xml create mode 100644 tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 4f56784..28ae92f 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -7664,7 +7664,7 @@ qemu-kvm -net nic,model=? /dev/null ... <devices> <tpm model='tpm-tis'> - <backend type='emulator'> + <backend type='emulator' version='2'> </backend> </tpm> </devices> @@ -7714,6 +7714,19 @@ qemu-kvm -net nic,model=? /dev/null </dd> </dl> </dd> + <dt><code>version</code></dt> + <dd> + <p> + The <code>version</code> attribute indicates the version + of the TPM. By default a TPM 1.2 is created. This attribute + only works with the <code>emulator</code> backend. The following + versions are supported: + </p> + <ul> + <li>'1.2' : creates a TPM 1.2</li> + <li>'2' : creates a TPM 2</li> + </ul> + </dd> </dl> <h4><a id="elementsNVRAM">NVRAM device</a></h4> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index a9a1020..6b31dd1 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -4143,6 +4143,18 @@ </attribute> </group> </choice> + <choice> + <group> + <optional> + <attribute name="version"> + <choice> + <value>1.2</value> + <value>2</value> + </choice> + </attribute> + </optional> + </group> + </choice> </element> </define> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 21b66d7..8e17af3 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -12594,7 +12594,7 @@ virDomainSmartcardDefParseXML(virDomainXMLOptionPtr xmlopt, * or like this: * * <tpm model='tpm-tis'> - * <backend type='emulator'/> + * <backend type='emulator' version='2'/> * </tpm> */ static virDomainTPMDefPtr @@ -12607,6 +12607,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, char *path = NULL; char *model = NULL; char *backend = NULL; + char *version = NULL; virDomainTPMDefPtr def; xmlNodePtr save = ctxt->node; xmlNodePtr *backends = NULL; @@ -12653,6 +12654,20 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, goto error; } + version = virXMLPropString(backends[0], "version"); + if (!version || STREQ(version, "1.2")) { + def->version = VIR_DOMAIN_TPM_VERSION_1_2; + /* only TIS available for emulator */ + if (def->type == VIR_DOMAIN_TPM_TYPE_EMULATOR) + def->model = VIR_DOMAIN_TPM_MODEL_TIS; + } else if (STREQ(version, "2")) { + def->version = VIR_DOMAIN_TPM_VERSION_2; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Unsupported TPM version '%s'"), + version); + } + switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: path = virXPathString("string(./backend/device/@path)", ctxt); @@ -12677,6 +12692,7 @@ virDomainTPMDefParseXML(virDomainXMLOptionPtr xmlopt, VIR_FREE(model); VIR_FREE(backend); VIR_FREE(backends); + VIR_FREE(version); ctxt->node = save; return def; @@ -21732,6 +21748,12 @@ virDomainTPMDefCheckABIStability(virDomainTPMDefPtr src, return false; } + if (src->version != dst->version) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Target TPM version doesn't match source")); + return false; + } + return virDomainDeviceInfoCheckABIStability(&src->info, &dst->info); } @@ -24834,6 +24856,9 @@ virDomainTPMDefFormat(virBufferPtr buf, virBufferAsprintf(buf, "<backend type='%s'", virDomainTPMBackendTypeToString(def->type)); + if (def->version == VIR_DOMAIN_TPM_VERSION_2) + virBufferAddLit(buf, " version='2'"); + switch (def->type) { case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: virBufferAddLit(buf, ">\n"); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index c304b08..223ba98 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1291,12 +1291,18 @@ typedef enum { VIR_DOMAIN_TPM_TYPE_LAST } virDomainTPMBackendType; +typedef enum { + VIR_DOMAIN_TPM_VERSION_1_2, + VIR_DOMAIN_TPM_VERSION_2, +} virDomainTPMVersion; + # define VIR_DOMAIN_TPM_DEFAULT_DEVICE "/dev/tpm0" struct _virDomainTPMDef { virDomainTPMBackendType type; virDomainDeviceInfo info; virDomainTPMModel model; + virDomainTPMVersion version; union { struct { virDomainChrSourceDef source; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 62f0146..26cc572 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -53,6 +53,41 @@ static char *swtpm_path; static char *swtpm_setup; static char *swtpm_ioctl; +bool swtpm_supports_tpm2; + +/* + * qemuTPMCheckForTPM2Support + * + * Check whether swtpm_setup supports TPM 2 + */ +static void +qemuTPMCheckForTPM2Support(void) +{ + virCommandPtr cmd; + char *help = NULL; + + if (!swtpm_setup) + return; + + cmd = virCommandNew(swtpm_setup); + if (!cmd) + return; + + virCommandAddArg(cmd, "--help"); + virCommandSetOutputBuffer(cmd, &help); + + if (virCommandRun(cmd, NULL) < 0) + goto cleanup; + + if (strstr(help, "--tpm2")) + swtpm_supports_tpm2 = true; + + cleanup: + virCommandFree(cmd); + VIR_FREE(help); +} + + /* * qemuTPMEmulatorInit * @@ -92,6 +127,7 @@ qemuTPMEmulatorInit(void) VIR_FREE(swtpm_setup); return -1; } + qemuTPMCheckForTPM2Support(); } if (!swtpm_ioctl) { @@ -119,16 +155,28 @@ qemuTPMEmulatorInit(void) * * @swtpmStorageDir: directory for swtpm persistent state * @vmname: The name of the VM for which to create the storage + * @tpmversion: version of the TPM * * Create the swtpm's storage path */ static char * qemuTPMCreateEmulatorStoragePath(const char *swtpmStorageDir, - const char *vmname) + const char *vmname, + virDomainTPMVersion tpmversion) { char *path = NULL; + const char *dir = ""; - ignore_value(virAsprintf(&path, "%s/%s/tpm1.2", swtpmStorageDir, vmname)); + switch (tpmversion) { + case VIR_DOMAIN_TPM_VERSION_1_2: + dir = "tpm1.2"; + break; + case VIR_DOMAIN_TPM_VERSION_2: + dir = "tpm2"; + break; + } + + ignore_value(virAsprintf(&path, "%s/%s/%s", swtpmStorageDir, vmname, dir)); return path; } @@ -289,7 +337,8 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm, if (!tpm->data.emulator.storagepath && !(tpm->data.emulator.storagepath = - qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr))) + qemuTPMCreateEmulatorStoragePath(swtpmStorageDir, uuidstr, + tpm->version))) return -1; return 0; @@ -513,6 +562,14 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, virCommandSetUID(cmd, swtpm_user); virCommandSetGID(cmd, swtpm_group); + switch (tpm->version) { + case VIR_DOMAIN_TPM_VERSION_1_2: + break; + case VIR_DOMAIN_TPM_VERSION_2: + virCommandAddArg(cmd, "--tpm2"); + break; + } + return cmd; error: diff --git a/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args b/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args new file mode 100644 index 0000000..82b676f --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.x86_64-latest.args @@ -0,0 +1,33 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +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 \ +-realtime mlock=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,path=/tmp/lib/domain--1-TPM-VM/monitor.sock,\ +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/qemuxml2argvdata/tpm-emulator-tpm2.xml b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml new file mode 100644 index 0000000..7546930 --- /dev/null +++ b/tests/qemuxml2argvdata/tpm-emulator-tpm2.xml @@ -0,0 +1,30 @@ +<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'/> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 26f9d70..e813698 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2003,6 +2003,7 @@ mymain(void) DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", 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_PARSE_ERROR("pci-domain-invalid", NONE); DO_TEST_PARSE_ERROR("pci-bus-invalid", NONE); diff --git a/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.xml new file mode 100644 index 0000000..eff55fc --- /dev/null +++ b/tests/qemuxml2xmloutdata/tpm-emulator-tpm2.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'> + <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'/> + </tpm> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/> + </memballoon> + </devices> +</domain> -- 2.5.5

Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM. To enable this we need to have the swtpm write its process id (pid) into a file. We then read it from the file to configure the emulator cgroup. The PID file is created in /var/run/libvirt/qemu/swtpm: [root@localhost swtpm]# ls -lZ /var/run/libvirt/qemu/swtpm/ total 4 -rw-r--r--. 1 tss tss system_u:object_r:qemu_var_run_t:s0 5 Apr 10 12:26 1-testvm-swtpm.pid srw-rw----. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 10 12:26 1-testvm-swtpm.sock The swtpm command line now looks as follows: root@localhost testvm]# ps auxZ | grep swtpm | grep socket | grep -v grep system_u:system_r:virtd_t:s0:c597,c632 tss 18697 0.0 0.0 28172 3892 ? Ss 16:46 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2/ --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --pid file=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.pid Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_cgroup.c | 35 ++++++++++++ src/qemu/qemu_cgroup.h | 2 + src/qemu/qemu_extdevice.c | 23 ++++++++ src/qemu/qemu_extdevice.h | 6 +++ src/qemu/qemu_process.c | 4 ++ src/qemu/qemu_tpm.c | 134 +++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_tpm.h | 6 +++ 7 files changed, 208 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1a5adca..c51062d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -26,6 +26,7 @@ #include "qemu_cgroup.h" #include "qemu_domain.h" #include "qemu_process.h" +#include "qemu_extdevice.h" #include "vircgroup.h" #include "virlog.h" #include "viralloc.h" @@ -1146,6 +1147,40 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, int +qemuSetupCgroupForExtDevices(virDomainObjPtr vm, + virQEMUDriverPtr driver) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup_temp = NULL; + int ret = -1; + + if (!qemuExtDevicesHasDevice(vm->def) || + priv->cgroup == NULL) + return 0; /* Not supported, so claim success */ + + /* + * If CPU cgroup controller is not initialized here, then we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, then there's nothing to do anyway. + */ + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) + return 0; + + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &cgroup_temp) < 0) + goto cleanup; + + ret = qemuExtDevicesSetupCgroup(driver, vm->def, cgroup_temp); + +cleanup: + virCgroupFree(&cgroup_temp); + + return ret; +} + + +int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 3b8ff60..c2fca7f 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -69,6 +69,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, long long quota); int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm); +int qemuSetupCgroupForExtDevices(virDomainObjPtr vm, + virQEMUDriverPtr driver); int qemuRemoveCgroup(virDomainObjPtr vm); typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData; diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 790b19b..dd66420 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -30,6 +30,8 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "virtpm.h" +#include "virpidfile.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -152,3 +154,24 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, if (def->tpm) qemuExtTPMStop(driver, def); } + + +bool +qemuExtDevicesHasDevice(virDomainDefPtr def) +{ + return def->tpm != NULL; +} + + +int +qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = 0; + + if (def->tpm) + ret = qemuExtTPMSetupCgroup(driver, def, cgroup); + + return ret; +} diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 6de858b..c557778 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -50,4 +50,10 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +bool qemuExtDevicesHasDevice(virDomainDefPtr def); + +int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup); + #endif /* __QEMU_EXTDEVICE_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9370de3..35a78f3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6075,6 +6075,10 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupEmulator(vm) < 0) goto cleanup; + VIR_DEBUG("Setting cgroup for external devices (if required)"); + if (qemuSetupCgroupForExtDevices(vm, driver) < 0) + goto cleanup; + VIR_DEBUG("Setting up resctrl"); if (qemuProcessResctrlCreate(driver, vm) < 0) goto cleanup; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 26cc572..da6acbf 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -39,6 +39,7 @@ #include "viruuid.h" #include "virfile.h" #include "virstring.h" +#include "virpidfile.h" #include "configmake.h" #include "qemu_tpm.h" @@ -346,6 +347,57 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm, /* + * qemuTPMCreatePidFilename + */ +static char * +qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir, + const char *shortName) +{ + char *pidfile = NULL; + char *devicename = NULL; + + if (virAsprintf(&devicename, "%s-swtpm", shortName) < 0) + return NULL; + + pidfile = virPidFileBuildPath(swtpmStateDir, devicename); + + VIR_FREE(devicename); + + return pidfile; +} + + +/* + * qemuTPMEmulatorGetPid + * + * @swtpmStateDir: the directory where swtpm writes the pidfile into + * @shortName: short name of the domain + * @pid: pointer to pid + * + * Return -errno upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuTPMEmulatorGetPid(const char *swtpmStateDir, + const char *shortName, + pid_t *pid) +{ + int ret; + char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, + shortName); + if (!pidfile) + return -ENOMEM; + + ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path); + + VIR_FREE(pidfile); + + return ret; +} + + +/* * qemuTPMEmulatorPrepareHost: * * @tpm: tpm definition @@ -514,6 +566,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath, * @privileged: whether we are running in privileged mode * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root) * @swtpm_group: The gid for the swtpm to run as + * @swtpmStateDir: the directory where swtpm writes the pid file and creates the + * Unix socket + * @shortName: the short name of the VM * * Create the virCommand use for starting the emulator * Do some initializations on the way, such as creation of storage @@ -525,10 +580,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, const unsigned char *vmuuid, bool privileged, uid_t swtpm_user, - gid_t swtpm_group) + gid_t swtpm_group, + const char *swtpmStateDir, + const char *shortName) { virCommandPtr cmd = NULL; bool created = false; + char *pidfile; if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, &created, swtpm_user, swtpm_group) < 0) @@ -570,6 +628,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, break; } + if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName))) + goto error; + + virCommandAddArg(cmd, "--pid"); + virCommandAddArgFormat(cmd, "file=%s", pidfile); + VIR_FREE(pidfile); + return cmd; error: @@ -721,6 +786,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg; virDomainTPMDefPtr tpm = def->tpm; char *shortName = virDomainDefGetShortName(def); + int timeout, rc; + pid_t pid; if (!shortName) return -1; @@ -733,7 +800,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid, driver->privileged, cfg->swtpm_user, - cfg->swtpm_group))) + cfg->swtpm_group, + cfg->swtpmStateDir, shortName))) goto cleanup; if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) @@ -769,6 +837,22 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, goto cleanup; } + /* check that the swtpm has written its pid into the file */ + timeout = 1000; /* ms */ + while (timeout > 0) { + rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); + if (rc < 0) { + timeout -= 50; + usleep(50 * 1000); + continue; + } + if (rc == 0 && pid == (pid_t)-1) + goto error; + break; + } + if (timeout <= 0) + goto error; + ret = 0; cleanup: @@ -781,6 +865,11 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virObjectUnref(cfg); return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("swtpm failed to start")); + goto cleanup; } @@ -830,3 +919,44 @@ qemuExtTPMStop(virQEMUDriverPtr driver, VIR_FREE(shortName); virObjectUnref(cfg); } + + +int +qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *pidfile = NULL; + char *shortName = NULL; + int ret = -1, rc; + pid_t pid; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + shortName = virDomainDefGetShortName(def); + if (!shortName) + goto cleanup; + rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); + if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get process id of swtpm")); + goto cleanup; + } + if (virCgroupAddTask(cgroup, pid) < 0) + goto cleanup; + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + ret = 0; + + cleanup: + VIR_FREE(pidfile); + VIR_FREE(shortName); + virObjectUnref(cfg); + + return ret; +} diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 20f3a9c..6eb1294 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -47,4 +47,10 @@ void qemuExtTPMStop(virQEMUDriverPtr driver, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + #endif /* __QEMU_TPM_H__ */ -- 2.5.5

On 05/10/2018 11:57 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM.
To enable this we need to have the swtpm write its process id (pid) into a file. We then read it from the file to configure the emulator cgroup.
The PID file is created in /var/run/libvirt/qemu/swtpm:
[root@localhost swtpm]# ls -lZ /var/run/libvirt/qemu/swtpm/ total 4 -rw-r--r--. 1 tss tss system_u:object_r:qemu_var_run_t:s0 5 Apr 10 12:26 1-testvm-swtpm.pid srw-rw----. 1 qemu qemu system_u:object_r:svirt_image_t:s0:c597,c632 0 Apr 10 12:26 1-testvm-swtpm.sock
The swtpm command line now looks as follows:
root@localhost testvm]# ps auxZ | grep swtpm | grep socket | grep -v grep system_u:system_r:virtd_t:s0:c597,c632 tss 18697 0.0 0.0 28172 3892 ? Ss 16:46 0:00 /usr/bin/swtpm socket --daemon --ctrl type=unixio,path=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.sock,mode=0600 --tpmstate dir=/var/lib/libvirt/swtpm/485d0004-a48f-436a-8457-8a3b73e28568/tpm1.2/ --log file=/var/log/swtpm/libvirt/qemu/testvm-swtpm.log --pid file=/var/run/libvirt/qemu/swtpm/1-testvm-swtpm.pid
Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com> --- src/qemu/qemu_cgroup.c | 35 ++++++++++++ src/qemu/qemu_cgroup.h | 2 + src/qemu/qemu_extdevice.c | 23 ++++++++ src/qemu/qemu_extdevice.h | 6 +++ src/qemu/qemu_process.c | 4 ++ src/qemu/qemu_tpm.c | 134 +++++++++++++++++++++++++++++++++++++++++++++- src/qemu/qemu_tpm.h | 6 +++ 7 files changed, 208 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c index 1a5adca..c51062d 100644 --- a/src/qemu/qemu_cgroup.c +++ b/src/qemu/qemu_cgroup.c @@ -26,6 +26,7 @@ #include "qemu_cgroup.h" #include "qemu_domain.h" #include "qemu_process.h" +#include "qemu_extdevice.h" #include "vircgroup.h" #include "virlog.h" #include "viralloc.h" @@ -1146,6 +1147,40 @@ qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup,
int +qemuSetupCgroupForExtDevices(virDomainObjPtr vm, + virQEMUDriverPtr driver) +{ + qemuDomainObjPrivatePtr priv = vm->privateData; + virCgroupPtr cgroup_temp = NULL; + int ret = -1; + + if (!qemuExtDevicesHasDevice(vm->def) || + priv->cgroup == NULL) + return 0; /* Not supported, so claim success */ + + /* + * If CPU cgroup controller is not initialized here, then we need + * neither period nor quota settings. And if CPUSET controller is + * not initialized either, then there's nothing to do anyway. + */ + if (!virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPU) && + !virCgroupHasController(priv->cgroup, VIR_CGROUP_CONTROLLER_CPUSET)) + return 0; + + if (virCgroupNewThread(priv->cgroup, VIR_CGROUP_THREAD_EMULATOR, 0, + false, &cgroup_temp) < 0) + goto cleanup; + + ret = qemuExtDevicesSetupCgroup(driver, vm->def, cgroup_temp); + +cleanup: make syntax-check wants a blank in front of the label
+ virCgroupFree(&cgroup_temp); + + return ret; +} + + +int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm) { qemuDomainObjPrivatePtr priv = vm->privateData; diff --git a/src/qemu/qemu_cgroup.h b/src/qemu/qemu_cgroup.h index 3b8ff60..c2fca7f 100644 --- a/src/qemu/qemu_cgroup.h +++ b/src/qemu/qemu_cgroup.h @@ -69,6 +69,8 @@ int qemuSetupCgroupVcpuBW(virCgroupPtr cgroup, long long quota); int qemuSetupCgroupCpusetCpus(virCgroupPtr cgroup, virBitmapPtr cpumask); int qemuSetupGlobalCpuCgroup(virDomainObjPtr vm); +int qemuSetupCgroupForExtDevices(virDomainObjPtr vm, + virQEMUDriverPtr driver); int qemuRemoveCgroup(virDomainObjPtr vm);
typedef struct _qemuCgroupEmulatorAllNodesData qemuCgroupEmulatorAllNodesData; diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c index 790b19b..dd66420 100644 --- a/src/qemu/qemu_extdevice.c +++ b/src/qemu/qemu_extdevice.c @@ -30,6 +30,8 @@ #include "virlog.h" #include "virstring.h" #include "virtime.h" +#include "virtpm.h" +#include "virpidfile.h"
#define VIR_FROM_THIS VIR_FROM_QEMU
@@ -152,3 +154,24 @@ qemuExtDevicesStop(virQEMUDriverPtr driver, if (def->tpm) qemuExtTPMStop(driver, def); } + + +bool +qemuExtDevicesHasDevice(virDomainDefPtr def) +{ + return def->tpm != NULL; +} + + +int +qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) +{ + int ret = 0; + + if (def->tpm) + ret = qemuExtTPMSetupCgroup(driver, def, cgroup); + + return ret; +} diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h index 6de858b..c557778 100644 --- a/src/qemu/qemu_extdevice.h +++ b/src/qemu/qemu_extdevice.h @@ -50,4 +50,10 @@ void qemuExtDevicesStop(virQEMUDriverPtr driver, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+bool qemuExtDevicesHasDevice(virDomainDefPtr def); + +int qemuExtDevicesSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup); + #endif /* __QEMU_EXTDEVICE_H__ */ diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 9370de3..35a78f3 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6075,6 +6075,10 @@ qemuProcessLaunch(virConnectPtr conn, if (qemuProcessSetupEmulator(vm) < 0) goto cleanup;
+ VIR_DEBUG("Setting cgroup for external devices (if required)"); + if (qemuSetupCgroupForExtDevices(vm, driver) < 0) + goto cleanup; + VIR_DEBUG("Setting up resctrl"); if (qemuProcessResctrlCreate(driver, vm) < 0) goto cleanup; diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c index 26cc572..da6acbf 100644 --- a/src/qemu/qemu_tpm.c +++ b/src/qemu/qemu_tpm.c @@ -39,6 +39,7 @@ #include "viruuid.h" #include "virfile.h" #include "virstring.h" +#include "virpidfile.h" #include "configmake.h" #include "qemu_tpm.h"
@@ -346,6 +347,57 @@ qemuTPMEmulatorInitPaths(virDomainTPMDefPtr tpm,
/* + * qemuTPMCreatePidFilename + */ +static char * +qemuTPMEmulatorCreatePidFilename(const char *swtpmStateDir, + const char *shortName) +{ + char *pidfile = NULL; + char *devicename = NULL; + + if (virAsprintf(&devicename, "%s-swtpm", shortName) < 0) + return NULL; + + pidfile = virPidFileBuildPath(swtpmStateDir, devicename); + + VIR_FREE(devicename); + + return pidfile; +} + + +/* + * qemuTPMEmulatorGetPid + * + * @swtpmStateDir: the directory where swtpm writes the pidfile into + * @shortName: short name of the domain + * @pid: pointer to pid + * + * Return -errno upon error, or zero on successful reading of the pidfile. + * If the PID was not still alive, zero will be returned, and @pid will be + * set to -1; + */ +static int +qemuTPMEmulatorGetPid(const char *swtpmStateDir, + const char *shortName, + pid_t *pid) +{ + int ret; + char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, + shortName); + if (!pidfile) + return -ENOMEM; + + ret = virPidFileReadPathIfAlive(pidfile, pid, swtpm_path); + + VIR_FREE(pidfile); + + return ret; +} + + +/* * qemuTPMEmulatorPrepareHost: * * @tpm: tpm definition @@ -514,6 +566,9 @@ qemuTPMEmulatorRunSetup(const char *storagepath, * @privileged: whether we are running in privileged mode * @swtpm_user: The uid for the swtpm to run as (drop privileges to from root) * @swtpm_group: The gid for the swtpm to run as + * @swtpmStateDir: the directory where swtpm writes the pid file and creates the + * Unix socket + * @shortName: the short name of the VM * * Create the virCommand use for starting the emulator * Do some initializations on the way, such as creation of storage @@ -525,10 +580,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, const unsigned char *vmuuid, bool privileged, uid_t swtpm_user, - gid_t swtpm_group) + gid_t swtpm_group, + const char *swtpmStateDir, + const char *shortName) { virCommandPtr cmd = NULL; bool created = false; + char *pidfile;
if (qemuTPMCreateEmulatorStorage(tpm->data.emulator.storagepath, &created, swtpm_user, swtpm_group) < 0) @@ -570,6 +628,13 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDefPtr tpm, break; }
+ if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName))) + goto error; + + virCommandAddArg(cmd, "--pid"); + virCommandAddArgFormat(cmd, "file=%s", pidfile); + VIR_FREE(pidfile); + return cmd;
error: @@ -721,6 +786,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virQEMUDriverConfigPtr cfg; virDomainTPMDefPtr tpm = def->tpm; char *shortName = virDomainDefGetShortName(def); + int timeout, rc; + pid_t pid;
if (!shortName) return -1; @@ -733,7 +800,8 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, if (!(cmd = qemuTPMEmulatorBuildCommand(tpm, def->name, def->uuid, driver->privileged, cfg->swtpm_user, - cfg->swtpm_group))) + cfg->swtpm_group, + cfg->swtpmStateDir, shortName))) goto cleanup;
if (qemuExtDeviceLogCommand(logCtxt, cmd, "TPM Emulator") < 0) @@ -769,6 +837,22 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, goto cleanup; }
+ /* check that the swtpm has written its pid into the file */ + timeout = 1000; /* ms */ + while (timeout > 0) { + rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); + if (rc < 0) { + timeout -= 50; + usleep(50 * 1000); + continue; + } + if (rc == 0 && pid == (pid_t)-1) + goto error; + break; + } + if (timeout <= 0) + goto error; + ret = 0;
cleanup: @@ -781,6 +865,11 @@ qemuExtTPMStartEmulator(virQEMUDriverPtr driver, virObjectUnref(cfg);
return ret; + + error: + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("swtpm failed to start")); + goto cleanup; }
@@ -830,3 +919,44 @@ qemuExtTPMStop(virQEMUDriverPtr driver, VIR_FREE(shortName); virObjectUnref(cfg); } + + +int +qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) +{ + virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + char *pidfile = NULL; + char *shortName = NULL; + int ret = -1, rc; + pid_t pid; + + switch (def->tpm->type) { + case VIR_DOMAIN_TPM_TYPE_EMULATOR: + shortName = virDomainDefGetShortName(def); + if (!shortName) + goto cleanup; + rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid); + if (rc < 0 || (rc == 0 && pid == (pid_t)-1)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not get process id of swtpm")); + goto cleanup; + } + if (virCgroupAddTask(cgroup, pid) < 0) + goto cleanup; + break; + case VIR_DOMAIN_TPM_TYPE_PASSTHROUGH: + case VIR_DOMAIN_TPM_TYPE_LAST: + break; + } + + ret = 0; + + cleanup: + VIR_FREE(pidfile); + VIR_FREE(shortName); + virObjectUnref(cfg); + + return ret; +} diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h index 20f3a9c..6eb1294 100644 --- a/src/qemu/qemu_tpm.h +++ b/src/qemu/qemu_tpm.h @@ -47,4 +47,10 @@ void qemuExtTPMStop(virQEMUDriverPtr driver, virDomainDefPtr def) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuExtTPMSetupCgroup(virQEMUDriverPtr driver, + virDomainDefPtr def, + virCgroupPtr cgroup) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) + ATTRIBUTE_RETURN_CHECK; + #endif /* __QEMU_TPM_H__ */
-- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/10/2018 05:57 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM.
I haven't made any changes to this yet. A possibility would be to put swtpm into its own tpm-emulator cgroup and extend the XML for the TPM to also have 'period' and 'quota': <tpm model='tpm-tis'> <backend type='emulator'> <period>1000</period> <quota>500</quota> </backend> </tpm> Or we add the following to cputune: <tpm_emulator_period>1000</tpm_emulator_period> <tpm_emulator_quota>500</tpm_emulator_quota> The latter would be more consistent, though i would prefer the former. Stefan

On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM.
I haven't made any changes to this yet. A possibility would be to put swtpm into its own tpm-emulator cgroup and extend the XML for the TPM to also have 'period' and 'quota':
<tpm model='tpm-tis'> <backend type='emulator'> <period>1000</period> <quota>500</quota> </backend> </tpm>
Or we add the following to cputune:
<tpm_emulator_period>1000</tpm_emulator_period> <tpm_emulator_quota>500</tpm_emulator_quota>
The latter would be more consistent, though i would prefer the former.
I'm not really seeing a compelling reason to need to set tunables on the swtpm directly. IMHO we should just consider it part of the "emulator" tunables - the fact that it is a separate binary/process rather than inside QEMU is just a private impl detail. 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 05/15/2018 11:34 AM, Daniel P. Berrangé wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM. I haven't made any changes to this yet. A possibility would be to put swtpm into its own tpm-emulator cgroup and extend the XML for the TPM to also have 'period' and 'quota':
<tpm model='tpm-tis'> <backend type='emulator'> <period>1000</period> <quota>500</quota> </backend> </tpm>
Or we add the following to cputune:
<tpm_emulator_period>1000</tpm_emulator_period> <tpm_emulator_quota>500</tpm_emulator_quota>
The latter would be more consistent, though i would prefer the former. I'm not really seeing a compelling reason to need to set tunables on
On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote: the swtpm directly. IMHO we should just consider it part of the "emulator" tunables - the fact that it is a separate binary/process rather than inside QEMU is just a private impl detail.
One reason I could think of is to approximate the real world a little closer where a TPM is typically its own chip. Stefan

On Tue, May 15, 2018 at 11:43:10AM -0400, Stefan Berger wrote:
On 05/15/2018 11:34 AM, Daniel P. Berrangé wrote:
On 05/10/2018 05:57 PM, Stefan Berger wrote:
Add the external swtpm to the emulator cgroup so that upper limits of CPU usage can be enforced on the emulated TPM. I haven't made any changes to this yet. A possibility would be to put swtpm into its own tpm-emulator cgroup and extend the XML for the TPM to also have 'period' and 'quota':
<tpm model='tpm-tis'> <backend type='emulator'> <period>1000</period> <quota>500</quota> </backend> </tpm>
Or we add the following to cputune:
<tpm_emulator_period>1000</tpm_emulator_period> <tpm_emulator_quota>500</tpm_emulator_quota>
The latter would be more consistent, though i would prefer the former. I'm not really seeing a compelling reason to need to set tunables on
On Tue, May 15, 2018 at 11:25:58AM -0400, Stefan Berger wrote: the swtpm directly. IMHO we should just consider it part of the "emulator" tunables - the fact that it is a separate binary/process rather than inside QEMU is just a private impl detail.
One reason I could think of is to approximate the real world a little closer where a TPM is typically its own chip.
I don't think that's a real use case. You can look at QEMU's machine types and say they have 10-20 separate chips in the real world, but we don't need to have control over CPU usage of those chips. 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 :|
participants (5)
-
Boris Fiuczynski
-
Daniel P. Berrangé
-
John Ferlan
-
Marc Hartmayer
-
Stefan Berger