2:13 p.m.
On 05/23/2018 02:03 PM, John Ferlan wrote:
On 05/23/2018 09:20 AM, Stefan Berger wrote:
> On 05/23/2018 08:07 AM, John Ferlan wrote:
>> On 05/22/2018 04:44 PM, Stefan Berger wrote:
>>> 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
>>>
>>> v5->v6:
>>> - Addressed John Ferlan's comments
>>> - rebased on latest tip
>>> - Added patch 12.
>>>
>>> v4->v5:
>>> - Addressed John Ferlan's, Boris Fiuczysnki's and Marc
Hartmayer's
>>> comments
>>> - rebased on latest tip
>>>
>>> v3->v4:
>>> - Addressed John Ferlan's comments
>>> - Fixed bugs I found while testing
>>> - rebased on latest tip
>>>
>>> Stefan Berger (12):
>>> 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
>>> conf: Add support for choosing emulation of a TPM 2
>>> qemu: Add swtpm to emulator cgroup
>>> news: Update news with new TPM emulator feature
>>>
>>> docs/formatdomain.html.in | 43 +
>>> docs/news.xml | 9 +
>>> 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 | 12 +
>>> 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_extdevice.c | 180 ++++
>>> src/qemu/qemu_extdevice.h | 59 ++
>>> src/qemu/qemu_process.c | 16 +
>>> src/qemu/qemu_security.c | 69 ++
>>> src/qemu/qemu_security.h | 11 +
>>> src/qemu/qemu_tpm.c | 946
>>> +++++++++++++++++++++
>>> 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 +
>>> 48 files changed, 2165 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
>>>
>> This all looks good to me - thanks for the news.xml adjustment. Barring
>> anyone else making a late/additional review - I will look to push the
>> series later on today.
> Thanks.
>
> As mentioned, I will need to follow up with AppArmor support. What I am
> currently experimenting with is a subprofile of the libvirt profile. The
> problem with it is it's 'suboptimal' in terms of 'unspecific'
paths
> containing wild-cards so that this single sub profile can accommodate
> the paths of all domains. This profile is static and not dynamically
> generated.
>
I see that Jano had some comments... I also was reminded today that you
still have libvirt.git commit access - so once you feel comfortable with
addressing those comments, I guess you have the capability to push and
won't need me for that!
Thanks. Between v3 and v4 it looks like some hunks got lost related to
the TPM 2 enablement. I readded them and added an error message in case
TPM 2 is not supported by swtpm_setup/swptm. I am keeping your
Reviewed-by's - no 'abuse' intended :-)
I'll defer to those with AppArmor experience for the rest!
Stefan
John
> diff --git a/examples/apparmor/usr.sbin.libvirtd
> b/examples/apparmor/usr.sbin.libvirtd
> index 3102cab382..dcab4feb6c 100644
> --- a/examples/apparmor/usr.sbin.libvirtd
> +++ b/examples/apparmor/usr.sbin.libvirtd
> @@ -126,4 +126,15 @@
>
> /usr/{lib,lib64,lib/qemu,libexec}/qemu-bridge-helper rmix,
> }
> + /usr/bin/swtpm Cx -> usr_bin_swtpm,
> + profile usr_bin_swtpm flags=(complain) {
> + #include <abstractions/base>
> +
> + /usr/bin/swtpm rm,
> +
> + /run/libvirt/qemu/swtpm/*-swtpm.pid rw,
> + /run/libvirt/qemu/swtpm/*-swtpm.sock w,
> + /var/log/swtpm/libvirt/qemu/*-swtpm.log w,
> +
>
/var/lib/libvirt/swtpm/[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*-[0-9a-f]*/{tpm1.2,tpm2}/{tpm,tpm2}-00.permall
> rw,
> + }
> }
>
>
> A better solution would be to extend the QEMU domain profile with these
> additional paths, which, as a side-effect, would give a QEMU instance
> access to these paths as well. Basically QEMU and swtpm would share that
> profile. To good thing is we can use specific paths (no wild cards) for
> the files that the swtpm needs to access.
>
> Yet a stricter solution would be to dynamically create a profile
> specifically for the swtpm that contains only the necessary paths.
> Though I think the code in src/security/security_apparmor.c is not
> prepared for that and it may end up being a bigger undertaking.
>
> Anyone who would be willing to give an opinion on the latter two cases ?
> I am cc'ing Christian Ehrhard who seems to have worked on AppArmor
> related code.
>
> Stefan
>
>> John
>>